From ee9b385ec5122038651cd3058350b03adef76ab2 Mon Sep 17 00:00:00 2001 From: Etienne Rossignon Date: Mon, 26 Oct 2026 11:16:43 +0100 Subject: [PATCH] fix: MCP stress findings: explore fan-out, auth check, output size, quality Explore walks breadth-first with one bulk /objects/related call per level (was one call per node, unbounded; one timeout aborted the walk), with a node budget and per-node errors; maxDepth 0 now means unlimited. Connect verifies the token on /v1/namespaces (/v1/info is public) and keeps the current token on re-connect. elementIds are deduplicated (max 100), tool results are capped with a narrowing hint, over-long suggestion queries skip fuzzy matching. Server side: a component history read failure now carries a 501 responseDetail instead of an empty series, and OPC UA status maps to quality by severity, so UncertainInitialValue (unused alarm sub-states) is no longer Bad and pseudo-session no longer leaks status names as quality. --- packages/core/src/domain/vqt.ts | 17 +- packages/core/src/helpers/opcua-mapping.ts | 22 +- packages/core/src/services/history-service.ts | 23 +- packages/core/tests/history-service.test.ts | 23 ++ packages/core/tests/opcua-mapping.test.ts | 26 +- packages/mcp/src/bindings/rest-binding.ts | 35 ++- packages/mcp/src/bindings/types.ts | 10 + packages/mcp/src/server.ts | 41 ++- packages/mcp/src/tools/connect.ts | 50 +++- packages/mcp/src/tools/explore-hierarchy.ts | 154 ++++++++--- packages/mcp/src/tools/monitor-events.ts | 23 +- packages/mcp/src/tools/query-telemetry.ts | 28 +- packages/mcp/src/utils/element-ids.ts | 16 ++ packages/mcp/src/utils/output-limit.ts | 29 +++ packages/mcp/src/utils/similarity.ts | 22 +- packages/mcp/tests/contract-stress.test.ts | 245 ++++++++++++++++++ packages/mcp/tests/contract.test.ts | 42 +-- .../mcp/tests/helpers/contract-fixture.ts | 47 ++++ packages/mcp/tests/mcp-server.test.ts | 42 +++ .../src/address-space-subscription.ts | 3 +- .../src/polling-subscription.ts | 3 +- .../src/pseudo-session-adapter.ts | 5 +- .../tests/pseudo-session-adapter.test.ts | 21 ++ 23 files changed, 799 insertions(+), 128 deletions(-) create mode 100644 packages/mcp/src/utils/element-ids.ts create mode 100644 packages/mcp/src/utils/output-limit.ts create mode 100644 packages/mcp/tests/contract-stress.test.ts create mode 100644 packages/mcp/tests/helpers/contract-fixture.ts diff --git a/packages/core/src/domain/vqt.ts b/packages/core/src/domain/vqt.ts index b37107b..7cc778a 100644 --- a/packages/core/src/domain/vqt.ts +++ b/packages/core/src/domain/vqt.ts @@ -25,7 +25,22 @@ export interface CurrentValueResult { export interface HistoricalValueResult { readonly isComposition: boolean; readonly values: readonly VQT[]; + /** + * Per-component series. A component whose history could not be read has no values + * and a `responseDetail` saying why (e.g. 501 not supported), so that it is not + * mistaken for an empty time range. + */ readonly components?: Readonly< - Record + Record< + string, + { + readonly values: readonly VQT[]; + readonly responseDetail?: { + readonly title: string; + readonly status: number; + readonly detail: string; + }; + } + > > | null; } diff --git a/packages/core/src/helpers/opcua-mapping.ts b/packages/core/src/helpers/opcua-mapping.ts index 2b753eb..ff59bfc 100644 --- a/packages/core/src/helpers/opcua-mapping.ts +++ b/packages/core/src/helpers/opcua-mapping.ts @@ -158,6 +158,22 @@ export function cleanOpcuaVariant(variant: any): unknown { return cleanOpcuaValue(variant); } +/** + * i3X quality of an OPC UA StatusCode, from its severity (top two bits): Good + * (00, including subcodes like GoodClamped), Uncertain (01, e.g. + * UncertainInitialValue: a node that never received a value) or Bad (10/11). + * A missing status code is Bad, as before. + */ +export function qualityOfStatusCode( + statusCode: number | undefined, +): 'Good' | 'Uncertain' | 'Bad' { + if (statusCode === undefined) return 'Bad'; + const severity = statusCode >>> 30; + if (severity === 0) return 'Good'; + if (severity === 1) return 'Uncertain'; + return 'Bad'; +} + /** Convert a DataValue-like object to a SourceDataValue. */ export function dataValueToSource(dv: { statusCode?: { value: number } | null; @@ -165,10 +181,9 @@ export function dataValueToSource(dv: { sourceTimestamp?: Date | null; serverTimestamp?: Date | null; }): SourceDataValue { - const isGood = dv.statusCode ? dv.statusCode.value === 0 : false; return { value: cleanOpcuaVariant(dv.value), - quality: isGood ? 'Good' : 'Bad', + quality: qualityOfStatusCode(dv.statusCode?.value), timestamp: dv.sourceTimestamp?.toISOString() ?? dv.serverTimestamp?.toISOString() ?? @@ -184,10 +199,9 @@ export function dataValueToHistorical(dv: { sourceTimestamp?: Date | null; serverTimestamp?: Date | null; }): SourceHistoricalValue { - const isGood = dv.statusCode ? dv.statusCode.value === 0 : false; return { value: cleanOpcuaVariant(dv.value), - quality: isGood ? 'Good' : 'Bad', + quality: qualityOfStatusCode(dv.statusCode?.value), timestamp: dv.sourceTimestamp?.toISOString() ?? dv.serverTimestamp?.toISOString() ?? diff --git a/packages/core/src/services/history-service.ts b/packages/core/src/services/history-service.ts index e90409f..0f69c3e 100644 --- a/packages/core/src/services/history-service.ts +++ b/packages/core/src/services/history-service.ts @@ -9,6 +9,17 @@ import type { ILogger } from '../ports/logger.js'; import type { BulkResultItem } from '../types/api.js'; import type { ModelService } from './model-service.js'; +type ComponentHistory = { + values: VQT[]; + responseDetail?: { title: string; status: number; detail: string }; +}; + +const HISTORY_NOT_SUPPORTED = { + title: 'Not Implemented', + status: 501, + detail: 'History read not supported', +} as const; + export class HistoryService { constructor( private readonly dataSource: IDataSourcePort, @@ -91,11 +102,7 @@ export class HistoryService { return { success: false, elementId, - responseDetail: { - title: 'Not Implemented', - status: 501, - detail: 'History read not supported', - }, + responseDetail: HISTORY_NOT_SUPPORTED, }; } }, @@ -112,7 +119,7 @@ export class HistoryService { endTime: Date, maxDepth: number, currentDepth: number, - ): Promise> { + ): Promise> { if (maxDepth > 0 && currentDepth >= maxDepth) return new Map(); const childIds = model.childrenById.get(parent.id) ?? []; @@ -131,7 +138,7 @@ export class HistoryService { } } - const result = new Map(); + const result = new Map(); // Read history for direct property children if (propSources.length > 0) { @@ -155,7 +162,7 @@ export class HistoryService { this._logger.debug( `readHistory failed for child property ${pid} (${sourceNodeId}): ${(err as Error).message}`, ); - result.set(pid, { values: [] }); + result.set(pid, { values: [], responseDetail: HISTORY_NOT_SUPPORTED }); } }), ); diff --git a/packages/core/tests/history-service.test.ts b/packages/core/tests/history-service.test.ts index d4591b4..f8a423b 100644 --- a/packages/core/tests/history-service.test.ts +++ b/packages/core/tests/history-service.test.ts @@ -153,4 +153,27 @@ describe('HistoryService', () => { expect(results2[0]!.result!.components![tempId]!.values).toHaveLength(2); expect(results2[0]!.result!.components![tempId]!.values[0]!.value).toBe(20.5); }); + + it('marks a component whose history read failed instead of an empty series', async () => { + const ds = mockDataSource(mockNodes, mockHistory); + const modelService = new ModelService(ds, nullLogger); + const historyService = new HistoryService(ds, modelService, nullLogger); + + const model = await modelService.getOrBuildModel(); + const rootId = stableI3xId(rootNsu, 'instance'); + const byName = (name: string) => + [...model.nodesById.values()].find((n) => n.name === name)!.id; + + const [root] = await historyService.readHistory([rootId], null, null, 2); + const components = root!.result!.components!; + expect(components[byName('ErrorNode')]).toEqual({ + values: [], + responseDetail: { + title: 'Not Implemented', + status: 501, + detail: 'History read not supported', + }, + }); + expect(components[byName('Temperature')]!.responseDetail).toBeUndefined(); + }); }); diff --git a/packages/core/tests/opcua-mapping.test.ts b/packages/core/tests/opcua-mapping.test.ts index a03b161..4aae63a 100644 --- a/packages/core/tests/opcua-mapping.test.ts +++ b/packages/core/tests/opcua-mapping.test.ts @@ -9,6 +9,7 @@ import { dataValueToSource, NODE_CLASS_NAMES, qualifiedNameToNsu, + qualityOfStatusCode, } from '../src/helpers/opcua-mapping.js'; describe('qualifiedNameToNsu', () => { @@ -140,7 +141,7 @@ describe('dataValueToHistorical', () => { it('marks bad historical value', () => { const result = dataValueToHistorical({ - statusCode: { value: 1 }, + statusCode: { value: 0x80000000 }, value: { value: null }, sourceTimestamp: null, serverTimestamp: null, @@ -238,3 +239,26 @@ describe('cleanOpcuaValue', () => { expect(cleanOpcuaValue([1, 'two', { x: 1 }])).toEqual([1, 'two', { x: 1 }]); }); }); + +describe('qualityOfStatusCode (OPC UA severity)', () => { + it.each([ + ['Good', 0x00000000, 'Good'], + ['GoodClamped, a Good subcode', 0x00300000, 'Good'], + ['UncertainInitialValue, never received a value', 0x40920000, 'Uncertain'], + ['UncertainLastUsableValue', 0x408f0000, 'Uncertain'], + ['BadNodeIdUnknown', 0x80340000, 'Bad'], + ['Bad', 0x80000000, 'Bad'], + ])('maps %s to %s', (_name, statusCode, quality) => { + expect(qualityOfStatusCode(statusCode)).toBe(quality); + expect(dataValueToSource({ statusCode: { value: statusCode } }).quality).toBe( + quality, + ); + expect(dataValueToHistorical({ statusCode: { value: statusCode } }).quality).toBe( + quality, + ); + }); + + it('keeps a missing status code Bad', () => { + expect(qualityOfStatusCode(undefined)).toBe('Bad'); + }); +}); diff --git a/packages/mcp/src/bindings/rest-binding.ts b/packages/mcp/src/bindings/rest-binding.ts index f779712..47d02f7 100644 --- a/packages/mcp/src/bindings/rest-binding.ts +++ b/packages/mcp/src/bindings/rest-binding.ts @@ -11,7 +11,7 @@ import type { ServerInfo, SuccessResponse, } from '@node-i3x/core'; -import { rankObjectSuggestions } from '../utils/similarity.js'; +import { MAX_FUZZY_QUERY_LENGTH, rankObjectSuggestions } from '../utils/similarity.js'; import type { ElementResult, I3XClientBinding, Suggestion } from './types.js'; /** How long the full object list used for suggestions is reused. */ @@ -22,6 +22,7 @@ export class RestClientBinding implements I3XClientBinding { private readonly headers: Record; private readonly requestTimeoutMs: number; private readonly fetchImpl?: typeof fetch; + private readonly token?: string; private objectCache?: { at: number; objects: Promise }; /** @@ -43,6 +44,7 @@ export class RestClientBinding implements I3XClientBinding { this.baseUrl = cleanUrl; this.requestTimeoutMs = timeoutMs; this.fetchImpl = fetchImpl; + this.token = token; this.headers = { 'Content-Type': 'application/json', Accept: 'application/json', @@ -52,6 +54,26 @@ export class RestClientBinding implements I3XClientBinding { } } + /** Normalized base URL, without a trailing /v1. */ + getBaseUrl(): string { + return this.baseUrl; + } + + get hasToken(): boolean { + return Boolean(this.token); + } + + /** + * A binding to `baseUrl`. Without `token`, the current token is kept when the URL + * is this binding's own (re-connecting must not silently drop the configured token). + */ + connectTo(baseUrl: string, token?: string): RestClientBinding { + // No fetchImpl: a connect targets the network, even from an in-process binding. + const next = new RestClientBinding(baseUrl, token, this.requestTimeoutMs); + if (token !== undefined || next.baseUrl !== this.baseUrl || !this.token) return next; + return new RestClientBinding(baseUrl, this.token, this.requestTimeoutMs); + } + private async request(path: string, options: RequestInit = {}): Promise { const url = `${this.baseUrl}${path}`; const signal = options.signal || AbortSignal.timeout(this.requestTimeoutMs); @@ -199,6 +221,16 @@ export class RestClientBinding implements I3XClientBinding { ); } + async getRelatedObjectsBulk( + elementIds: string[], + ): Promise[]> { + return this.postBulkPerElement( + '/v1/objects/related', + elementIds, + {}, + ); + } + async getObjectValues( elementIds: string[], maxDepth = 1, @@ -223,6 +255,7 @@ export class RestClientBinding implements I3XClientBinding { /** Closest objects by elementId or displayName; a failure to list objects propagates. */ async findSuggestions(query: string): Promise { + if (query.trim().length > MAX_FUZZY_QUERY_LENGTH) return []; const now = Date.now(); if (!this.objectCache || now - this.objectCache.at > OBJECT_CACHE_TTL_MS) { const objects = this.getObjects(); diff --git a/packages/mcp/src/bindings/types.ts b/packages/mcp/src/bindings/types.ts index c1fe6e1..f848fee 100644 --- a/packages/mcp/src/bindings/types.ts +++ b/packages/mcp/src/bindings/types.ts @@ -43,6 +43,12 @@ export interface HierarchyNode { displayName?: string; typeElementId?: string; children?: HierarchyNode[]; + /** Children could not be read; the rest of the tree is unaffected. */ + error?: string; + /** Already expanded elsewhere in this tree (shared component or cycle). */ + repeated?: true; + /** Children not fetched because the node budget was reached. */ + truncated?: true; } export interface TelemetrySummary { @@ -72,6 +78,10 @@ export interface I3XClientBinding { elementId: string, relationshipType?: string, ): Promise; + /** Related objects of several elements in one request, one entry per elementId. */ + getRelatedObjectsBulk( + elementIds: string[], + ): Promise[]>; /** One entry per requested elementId, in request order. */ getObjectValues( elementIds: string[], diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 6d097fe..039b291 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -18,6 +18,7 @@ import { import { handleInspectAsset, inspectAssetSchema } from './tools/inspect-asset.js'; import { handleMonitorEvents, monitorEventsSchema } from './tools/monitor-events.js'; import { handleQueryTelemetry, queryTelemetrySchema } from './tools/query-telemetry.js'; +import { DEFAULT_MAX_RESULT_CHARS, serializeToolResult } from './utils/output-limit.js'; const { version } = createRequire(import.meta.url)('../package.json') as { version: string; @@ -27,8 +28,17 @@ export class NodeI3xMcpServer { private readonly server: Server; private binding: I3XClientBinding; - constructor(binding?: I3XClientBinding) { + private readonly maxResultChars: number; + + /** + * @param options.maxResultChars - cap on a serialized tool result (default + * I3X_MCP_MAX_RESULT_CHARS, else 100 000 characters). + */ + constructor(binding?: I3XClientBinding, options: { maxResultChars?: number } = {}) { this.binding = binding || new RestClientBinding(); + this.maxResultChars = + options.maxResultChars ?? + (Number(process.env.I3X_MCP_MAX_RESULT_CHARS) || DEFAULT_MAX_RESULT_CHARS); this.server = new Server( { @@ -72,12 +82,13 @@ export class NodeI3xMcpServer { }, authToken: { type: 'string', - description: 'Optional Bearer authentication token for secured endpoints', + description: + 'Optional Bearer authentication token. Omit it to keep the current token when re-connecting to the same URL.', }, verifyConnection: { type: 'boolean', description: - 'Whether to verify connectivity by querying server /v1/info (default: true)', + 'Whether to verify connectivity and credentials before switching (default: true); a rejected token returns status auth_failed', }, }, required: ['serverUrl'], @@ -129,7 +140,12 @@ export class NodeI3xMcpServer { maxDepth: { type: 'number', description: - 'Maximum depth to traverse child components (1 to 5, default: 2)', + 'Levels to show: 1 = roots only, 2 = roots and their children (default), 0 = unlimited (bounded by maxNodes)', + }, + maxNodes: { + type: 'number', + description: + 'Node budget for the whole tree (default 500, max 5000); the result is marked truncated when reached', }, }, }, @@ -145,7 +161,7 @@ export class NodeI3xMcpServer { type: 'array', items: { type: 'string' }, description: - "Array of asset elementIds to query (e.g. ['Pump_101', 'Boiler_101'])", + "Array of asset elementIds to query (e.g. ['Pump_101', 'Boiler_101']); duplicates are ignored, at most 100", }, mode: { type: 'string', @@ -207,7 +223,8 @@ export class NodeI3xMcpServer { elementIds: { type: 'array', items: { type: 'string' }, - description: 'Array of asset elementIds to check', + description: + 'Array of asset elementIds to check; duplicates are ignored, at most 100', }, maxDepth: { type: 'number', @@ -231,9 +248,13 @@ export class NodeI3xMcpServer { switch (name) { case 'i3x_connect': { const parsed = connectSchema.parse(rawArgs); - result = await handleConnect(parsed, (newBinding) => { - this.binding = newBinding; - }); + result = await handleConnect( + parsed, + (newBinding) => { + this.binding = newBinding; + }, + this.binding, + ); break; } case 'i3x_inspect_asset': { @@ -269,7 +290,7 @@ export class NodeI3xMcpServer { content: [ { type: 'text', - text: JSON.stringify(result, null, 2), + text: serializeToolResult(name, result, this.maxResultChars), }, ], }; diff --git a/packages/mcp/src/tools/connect.ts b/packages/mcp/src/tools/connect.ts index e4fdc06..05aa738 100644 --- a/packages/mcp/src/tools/connect.ts +++ b/packages/mcp/src/tools/connect.ts @@ -11,21 +11,31 @@ export const connectSchema = z.object({ authToken: z .string() .optional() - .describe('Optional Bearer authentication token for secured endpoints'), + .describe( + 'Optional Bearer authentication token. Omitted when re-connecting to the current URL: the current token is kept.', + ), verifyConnection: z .boolean() .optional() .default(true) - .describe('Whether to immediately verify connectivity by querying server /v1/info'), + .describe( + 'Whether to verify connectivity and credentials before switching (reads /v1/info and /v1/namespaces)', + ), }); export async function handleConnect( args: z.infer, onBindingSwitch: (newBinding: I3XClientBinding) => void, + current?: I3XClientBinding, ) { const { serverUrl, authToken, verifyConnection } = args; - const newBinding = new RestClientBinding(serverUrl, authToken); + const newBinding = + current instanceof RestClientBinding + ? current.connectTo(serverUrl, authToken) + : new RestClientBinding(serverUrl, authToken); + const tokenKept = authToken === undefined && newBinding.hasToken; + const url = newBinding.getBaseUrl(); let serverInfo: unknown; if (verifyConnection) { @@ -34,9 +44,30 @@ export async function handleConnect( } catch (err) { return { status: 'connection_failed', - serverUrl, + serverUrl: url, error: (err as Error).message, - tip: 'Please check that the server URL is reachable and the authentication token (if required) is valid.', + tip: 'Please check that the server URL is reachable.', + }; + } + // /v1/info is public on secured servers, so it proves nothing about the token: + // check credentials on an endpoint that requires them. + try { + await newBinding.getNamespaces(); + } catch (err) { + const message = (err as Error).message; + if (/^HTTP 40[13]\b/.test(message)) { + return { + status: 'auth_failed', + serverUrl: url, + error: message, + tip: 'The server rejected the credentials. Pass a valid authToken; the previous connection is still active.', + }; + } + return { + status: 'connection_failed', + serverUrl: url, + error: message, + tip: 'The server answered /v1/info but not /v1/namespaces; the previous connection is still active.', }; } } @@ -46,9 +77,12 @@ export async function handleConnect( return { status: 'connected', - serverUrl, - authenticated: Boolean(authToken), + serverUrl: url, + authenticated: newBinding.hasToken, + ...(tokenKept + ? { note: 'No authToken given: kept the token already in use for this URL.' } + : {}), serverInfo: serverInfo || 'Connection established without verification.', - message: `Successfully connected to ${serverUrl}. All subsequent i3X tool calls will query this endpoint.`, + message: `Successfully connected to ${url}. All subsequent i3X tool calls will query this endpoint.`, }; } diff --git a/packages/mcp/src/tools/explore-hierarchy.ts b/packages/mcp/src/tools/explore-hierarchy.ts index b302107..f3dcf69 100644 --- a/packages/mcp/src/tools/explore-hierarchy.ts +++ b/packages/mcp/src/tools/explore-hierarchy.ts @@ -2,6 +2,11 @@ import { z } from 'zod'; import type { HierarchyNode, I3XClientBinding } from '../bindings/types.js'; import { isParentRelationship } from '../utils/relationships.js'; +/** elementIds per /objects/related request: bounds body size and server work per call. */ +const RELATED_BATCH_SIZE = 100; +/** Concurrent /objects/related requests while expanding one level. */ +const RELATED_CONCURRENCY = 4; + export const exploreHierarchySchema = z.object({ rootElementId: z .string() @@ -11,19 +16,35 @@ export const exploreHierarchySchema = z.object({ ), maxDepth: z .number() + .int() + .min(0) .optional() .default(2) - .describe('Maximum depth to traverse child components (1 to 5, default: 2)'), + .describe( + 'Levels to show: 1 = roots only, 2 = roots and their children (default), 0 = unlimited (bounded by maxNodes)', + ), + maxNodes: z + .number() + .int() + .min(1) + .max(5000) + .optional() + .default(500) + .describe( + 'Node budget for the whole tree (default 500, max 5000); the tree is marked truncated when reached', + ), }); +type Asset = { elementId: string; displayName?: string; typeElementId?: string }; + export async function handleExploreHierarchy( binding: I3XClientBinding, args: z.infer, ) { - const maxDepth = Math.min(Math.max(args.maxDepth || 2, 1), 5); - - let roots: Array<{ elementId: string; displayName?: string; typeElementId?: string }>; + const maxDepth = args.maxDepth ?? 2; + const maxNodes = args.maxNodes ?? 500; + let roots: Asset[]; if (args.rootElementId) { const root = await binding.getObjectById(args.rootElementId); if (!root) { @@ -38,47 +59,106 @@ export async function handleExploreHierarchy( roots = await binding.getObjects({ rootOnly: true }); } - async function buildNode( - asset: { elementId: string; displayName?: string; typeElementId?: string }, - currentDepth: number, - visited: Set, - ): Promise { - const node: HierarchyNode = { - elementId: asset.elementId, - displayName: asset.displayName || asset.elementId, - typeElementId: asset.typeElementId, - }; - - visited.add(asset.elementId); + const toNode = (asset: Asset): HierarchyNode => ({ + elementId: asset.elementId, + displayName: asset.displayName || asset.elementId, + typeElementId: asset.typeElementId, + }); - if (currentDepth < maxDepth) { - // 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) => !visited.has(c.elementId)); + // Breadth-first, one bulk /objects/related call per batch of a level, instead of + // one call per node: per-node fan-out timed out on large address spaces. + let nodeCount = 0; + let truncated = false; + const expanded = new Set(); + const tree: HierarchyNode[] = []; + for (const root of roots) { + if (nodeCount >= maxNodes) { + truncated = true; + break; + } + tree.push(toNode(root)); + nodeCount++; + } - if (unvisitedChildren.length > 0) { - node.children = await Promise.all( - unvisitedChildren.map((child) => - buildNode(child, currentDepth + 1, new Set(visited)), - ), - ); + let level = tree; + for (let depth = 1; level.length > 0 && (maxDepth === 0 || depth < maxDepth); depth++) { + // Expand each element once; a shared component or a cycle shows up as `repeated`. + const toExpand: HierarchyNode[] = []; + for (const node of level) { + if (expanded.has(node.elementId)) node.repeated = true; + else { + expanded.add(node.elementId); + toExpand.push(node); } } - return node; - } + const related = await readRelatedInBatches( + binding, + toExpand.map((n) => n.elementId), + ); - const hierarchyTree = await Promise.all( - roots.map((root) => buildNode(root, 1, new Set())), - ); + const next: HierarchyNode[] = []; + toExpand.forEach((node, idx) => { + const entry = related[idx]; + if (entry?.error) { + node.error = entry.error; + return; + } + const children = (entry?.result ?? []) + .filter((r) => !isParentRelationship(r.sourceRelationship)) + .map((r) => r.object); + if (children.length === 0) return; + if (nodeCount + children.length > maxNodes) { + truncated = true; + node.truncated = true; + return; + } + node.children = children.map(toNode); + nodeCount += children.length; + next.push(...node.children); + }); + level = next; + } return { totalRoots: roots.length, - traversalDepth: maxDepth, - plantTopology: hierarchyTree, + traversalDepth: maxDepth === 0 ? 'unlimited' : maxDepth, + nodeCount, + ...(truncated + ? { + truncated: { + maxNodes, + hint: 'Node budget reached; nodes marked truncated were not expanded. Use rootElementId on one of them, a smaller maxDepth, or a larger maxNodes.', + }, + } + : {}), + plantTopology: tree, }; } + +async function readRelatedInBatches(binding: I3XClientBinding, elementIds: string[]) { + const batches: string[][] = []; + for (let i = 0; i < elementIds.length; i += RELATED_BATCH_SIZE) { + batches.push(elementIds.slice(i, i + RELATED_BATCH_SIZE)); + } + const results: Awaited>[] = + new Array(batches.length); + let nextBatch = 0; + const worker = async () => { + while (nextBatch < batches.length) { + const idx = nextBatch++; + const batch = batches[idx]!; + try { + results[idx] = await binding.getRelatedObjectsBulk(batch); + } catch (err) { + // A failed request marks its own nodes only; the rest of the walk goes on. + const error = err instanceof Error ? err.message : String(err); + results[idx] = batch.map((elementId) => ({ elementId, error })); + } + } + }; + await Promise.all( + Array.from({ length: Math.min(RELATED_CONCURRENCY, batches.length) }, worker), + ); + return results.flat(); +} diff --git a/packages/mcp/src/tools/monitor-events.ts b/packages/mcp/src/tools/monitor-events.ts index c3771f2..46bffa0 100644 --- a/packages/mcp/src/tools/monitor-events.ts +++ b/packages/mcp/src/tools/monitor-events.ts @@ -1,11 +1,14 @@ import { z } from 'zod'; import type { I3XClientBinding } from '../bindings/types.js'; +import { elementIdsSchema } from '../utils/element-ids.js'; + +/** Alerts listed one by one; the summary still counts all of them. */ +const MAX_LISTED_ALERTS = 50; export const monitorEventsSchema = z.object({ - elementIds: z - .array(z.string()) - .min(1) - .describe("Array of asset elementIds to check (e.g. ['Boiler_101', 'Pump_101'])"), + elementIds: elementIdsSchema( + "Array of asset elementIds to check (e.g. ['Boiler_101', 'Pump_101'])", + ), maxDepth: z .number() .int() @@ -43,10 +46,20 @@ export async function handleMonitorEvents( } } + const summary = { + bad: alerts.filter((a) => a.quality && /^bad/i.test(a.quality)).length, + uncertain: alerts.filter((a) => a.quality && /^uncertain/i.test(a.quality)).length, + readErrors: alerts.filter((a) => a.error).length, + }; + return { status: 'snapshot', checkedAt: new Date().toISOString(), - alerts, + summary, + alerts: alerts.slice(0, MAX_LISTED_ALERTS), + ...(alerts.length > MAX_LISTED_ALERTS + ? { alertsOmitted: alerts.length - MAX_LISTED_ALERTS } + : {}), snapshot, note: 'Point-in-time read; no subscription is created. Call again to re-check.', }; diff --git a/packages/mcp/src/tools/query-telemetry.ts b/packages/mcp/src/tools/query-telemetry.ts index a4072ef..056c3ef 100644 --- a/packages/mcp/src/tools/query-telemetry.ts +++ b/packages/mcp/src/tools/query-telemetry.ts @@ -1,12 +1,12 @@ import type { VQT } from '@node-i3x/core'; import { z } from 'zod'; import type { I3XClientBinding } from '../bindings/types.js'; +import { elementIdsSchema } from '../utils/element-ids.js'; export const queryTelemetrySchema = z.object({ - elementIds: z - .array(z.string()) - .min(1) - .describe("Array of asset elementIds to query (e.g. ['Pump_101', 'Boiler_101'])"), + elementIds: elementIdsSchema( + "Array of asset elementIds to query (e.g. ['Pump_101', 'Boiler_101'])", + ), mode: z .enum(['current', 'history']) .optional() @@ -78,6 +78,10 @@ export function parseTime( return parsed.toISOString(); } +function describeDetail(d: { status?: number; title?: string; detail?: string }): string { + return [d.status, d.title, d.detail].filter(Boolean).join(' '); +} + /** 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'); @@ -155,12 +159,24 @@ export async function handleQueryTelemetry( statistics: seriesStatistics(result.values ?? []), }; if (result.components) { + const entries = Object.entries(result.components); report.components = Object.fromEntries( - Object.entries(result.components).map(([componentId, { values }]) => [ + entries.map(([componentId, { values, responseDetail }]) => [ componentId, - { dataPoints: values ?? [], statistics: seriesStatistics(values ?? []) }, + // A failed component read is not an empty time range: report why. + responseDetail + ? { error: describeDetail(responseDetail) } + : { dataPoints: values ?? [], statistics: seriesStatistics(values ?? []) }, ]), ); + const failed = entries.filter(([, c]) => c.responseDetail); + if ( + failed.length > 0 && + failed.length === entries.length && + !result.values?.length + ) { + report.note = `No component history could be read (${describeDetail(failed[0]![1].responseDetail!)}): the server keeps no history for this asset's variables.`; + } } return report; }); diff --git a/packages/mcp/src/utils/element-ids.ts b/packages/mcp/src/utils/element-ids.ts new file mode 100644 index 0000000..3d5526b --- /dev/null +++ b/packages/mcp/src/utils/element-ids.ts @@ -0,0 +1,16 @@ +import { z } from 'zod'; + +/** Distinct elementIds per call: bounds request size and the tool result. */ +export const MAX_ELEMENT_IDS = 100; + +/** elementIds, deduplicated in order: repeated ids only multiply the output. */ +export function elementIdsSchema(description: string) { + return z + .array(z.string()) + .min(1) + .transform((ids) => [...new Set(ids)]) + .refine((ids) => ids.length <= MAX_ELEMENT_IDS, { + message: `At most ${MAX_ELEMENT_IDS} distinct elementIds per call.`, + }) + .describe(`${description}; duplicates are ignored, at most ${MAX_ELEMENT_IDS}`); +} diff --git a/packages/mcp/src/utils/output-limit.ts b/packages/mcp/src/utils/output-limit.ts new file mode 100644 index 0000000..2318782 --- /dev/null +++ b/packages/mcp/src/utils/output-limit.ts @@ -0,0 +1,29 @@ +/** Default cap on a serialized tool result, in characters (I3X_MCP_MAX_RESULT_CHARS). */ +export const DEFAULT_MAX_RESULT_CHARS = 100_000; + +const NARROWING_HINTS: Record = { + i3x_explore_plant_hierarchy: 'pass rootElementId, or lower maxDepth or maxNodes', + i3x_inspect_asset: 'lower maxDepth, or set includeSchema to false', + i3x_query_telemetry: 'pass fewer elementIds, lower maxDepth, or shorten the time range', + i3x_discover_types_and_schemas: 'pass typeElementId or namespaceUri', + i3x_monitor_asset_events: 'pass fewer elementIds or lower maxDepth', +}; + +/** + * Serialize a tool result, cutting it at `maxChars` with an explicit notice of what + * was cut and how to narrow the query, instead of flooding the client's context. + */ +export function serializeToolResult( + tool: string, + result: unknown, + maxChars: number, +): string { + const text = JSON.stringify(result, null, 2); + if (text.length <= maxChars) return text; + const hint = NARROWING_HINTS[tool] ?? 'narrow the query'; + return ( + `${text.slice(0, maxChars)}\n\n` + + `[Result truncated: showing the first ${maxChars} of ${text.length} characters, ` + + `so the JSON above is incomplete. To see everything, ${hint}.]` + ); +} diff --git a/packages/mcp/src/utils/similarity.ts b/packages/mcp/src/utils/similarity.ts index f16200a..8b40eb1 100644 --- a/packages/mcp/src/utils/similarity.ts +++ b/packages/mcp/src/utils/similarity.ts @@ -57,6 +57,23 @@ export function rankSuggestions( .map((item) => item.candidate); } +/** Longer queries are not ids or names someone typed: no fuzzy matching (O(n·m) each). */ +export const MAX_FUZZY_QUERY_LENGTH = 128; + +/** + * computeSimilarity, or 0 without running Levenshtein when the length difference + * alone keeps the score under `minScore` (distance >= |m - n|) and neither string + * contains the other. + */ +function boundedSimilarity(query: string, candidate: string, minScore: number): number { + const q = query.toLowerCase().trim(); + const c = candidate.toLowerCase().trim(); + if (!q || !c) return 0; + if (q.includes(c) || c.includes(q)) return computeSimilarity(q, c); + const lengthBound = 1 - Math.abs(q.length - c.length) / Math.max(q.length, c.length); + return lengthBound < minScore ? 0 : computeSimilarity(q, c); +} + /** * 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. @@ -67,13 +84,14 @@ export function rankObjectSuggestions( topN = 5, minScore = 0.4, ): Array<{ elementId: string; displayName?: string }> { + if (query.trim().length > MAX_FUZZY_QUERY_LENGTH) return []; return objects .map(({ elementId, displayName }) => ({ elementId, displayName, score: Math.max( - computeSimilarity(query, elementId), - displayName ? computeSimilarity(query, displayName) : 0, + boundedSimilarity(query, elementId, minScore), + displayName ? boundedSimilarity(query, displayName, minScore) : 0, ), })) .filter((item) => item.score >= minScore) diff --git a/packages/mcp/tests/contract-stress.test.ts b/packages/mcp/tests/contract-stress.test.ts new file mode 100644 index 0000000..45c35b8 --- /dev/null +++ b/packages/mcp/tests/contract-stress.test.ts @@ -0,0 +1,245 @@ +// Contract tests for the stress-test findings: explore fan-out, connect token check, +// output size, suggestion cost and history errors, against the real rest-server +// routes in-process through inject() (no port). +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; +import { createApp } from '@node-i3x/rest-server'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { RestClientBinding } from '../src/bindings/rest-binding.js'; +import { InProcessConnectorBinding, injectFetch } from '../src/in-process.js'; +import { NodeI3xMcpServer } from '../src/server.js'; +import { handleConnect } from '../src/tools/connect.js'; +import { handleExploreHierarchy } from '../src/tools/explore-hierarchy.js'; +import { + handleQueryTelemetry, + queryTelemetrySchema, +} from '../src/tools/query-telemetry.js'; +import { rankObjectSuggestions } from '../src/utils/similarity.js'; +import { makeDeps } from './helpers/contract-fixture.js'; + +type Injectable = Awaited>; + +/** fetch into an app, forwarding headers (unlike injectFetch, auth stays on). */ +function appFetch(app: Injectable): typeof fetch { + return 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}`, + headers: init?.headers as Record | undefined, + payload: typeof init?.body === 'string' ? init.body : undefined, + }); + return new Response(res.body, { status: res.statusCode }); + }; +} + +async function idsByName(binding: RestClientBinding) { + const all = await binding.getObjects(); + return Object.fromEntries(all.map((o) => [o.displayName, o.elementId])); +} + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('explore fan-out', () => { + it('walks level by level with one bulk /objects/related call per level', async () => { + const inject = injectFetch(makeDeps()); + const relatedBodies: Array<{ elementIds: string[] }> = []; + const counting: typeof fetch = (input, init) => { + if (String(input).endsWith('/v1/objects/related')) { + relatedBodies.push(JSON.parse(String(init?.body))); + } + return inject(input, init); + }; + const binding = new RestClientBinding( + 'http://i3x.example', + undefined, + 15_000, + counting, + ); + + // maxDepth 0 = unlimited, as in the value tools (it used to become 2). + const res = await handleExploreHierarchy(binding, { maxDepth: 0, maxNodes: 500 }); + const machine = res.plantTopology?.[0]; + const motor = machine?.children?.find((c) => c.displayName === 'Motor'); + expect(motor?.children?.map((c) => c.displayName)).toEqual(['Speed']); + + // Machine | Temperature, Reset, Motor | Speed: 3 levels, 3 requests (not 5). + expect(relatedBodies.map((b) => b.elementIds.length)).toEqual([1, 3, 1]); + expect(res).toMatchObject({ traversalDepth: 'unlimited', nodeCount: 5 }); + }); + + it('records a failed node on the node and keeps walking the rest', async () => { + const deps = makeDeps(); + const ids = await idsByName(new InProcessConnectorBinding(deps)); + const inject = injectFetch(deps); + // The server fails Motor's item only, as a per-item bulk error. + const failingMotor: typeof fetch = async (input, init) => { + const res = await inject(input, init); + if (!String(input).endsWith('/v1/objects/related')) return res; + const body = await res.json(); + body.results = body.results.map((r: { elementId: string }) => + r.elementId === ids.Motor + ? { + success: false, + elementId: r.elementId, + responseDetail: { title: 'Error', status: 500, detail: 'boom' }, + } + : r, + ); + return new Response(JSON.stringify(body), { status: 200 }); + }; + const binding = new RestClientBinding( + 'http://i3x.example', + undefined, + 15_000, + failingMotor, + ); + + const res = await handleExploreHierarchy(binding, { maxDepth: 0, maxNodes: 500 }); + const children = res.plantTopology?.[0]?.children ?? []; + expect(children.map((c) => c.displayName)).toEqual(['Temperature', 'Reset', 'Motor']); + expect(children.find((c) => c.displayName === 'Motor')).toMatchObject({ + error: '500 Error boom', + }); + }); + + it('stops at the node budget and says so', async () => { + const binding = new InProcessConnectorBinding(makeDeps()); + const res = await handleExploreHierarchy(binding, { maxDepth: 0, maxNodes: 3 }); + expect(res.plantTopology?.[0]).toMatchObject({ + displayName: 'Machine', + truncated: true, + }); + expect(res.truncated).toMatchObject({ maxNodes: 3 }); + expect(res.nodeCount).toBe(1); + }); +}); + +describe('i3x_connect credential check', () => { + async function securedServer() { + const app = await createApp(makeDeps({ apiKey: 'secret', requireAuth: true }), { + logger: false, + }); + vi.stubGlobal('fetch', appFetch(app)); + } + + it('reports auth_failed for a wrong token and keeps the previous connection', async () => { + await securedServer(); + const onSwitch = vi.fn(); + const res = await handleConnect( + { serverUrl: 'http://i3x.example', authToken: 'wrong', verifyConnection: true }, + onSwitch, + ); + expect(res).toMatchObject({ status: 'auth_failed' }); + expect(onSwitch).not.toHaveBeenCalled(); + + const ok = await handleConnect( + { serverUrl: 'http://i3x.example', authToken: 'secret', verifyConnection: true }, + onSwitch, + ); + expect(ok).toMatchObject({ status: 'connected', authenticated: true }); + }); + + it('keeps the current token when re-connecting to the same URL without one', async () => { + await securedServer(); + const current = new RestClientBinding('http://i3x.example/v1', 'secret'); + const res = await handleConnect( + { serverUrl: 'http://i3x.example', verifyConnection: true }, + () => {}, + current, + ); + expect(res).toMatchObject({ status: 'connected', authenticated: true }); + expect(res).toHaveProperty('note'); + }); +}); + +describe('output size', () => { + it('deduplicates elementIds and bounds their count', async () => { + const binding = new InProcessConnectorBinding(makeDeps()); + const { Motor } = await idsByName(binding); + const args = queryTelemetrySchema.parse({ + elementIds: Array.from({ length: 200 }, () => Motor), + mode: 'current', + }); + expect(args.elementIds).toEqual([Motor]); + const res = await handleQueryTelemetry(binding, args); + expect(res.telemetry).toHaveLength(1); + + expect(() => + queryTelemetrySchema.parse({ + elementIds: Array.from({ length: 101 }, (_, i) => `id-${i}`), + }), + ).toThrow(/At most 100 distinct elementIds/); + }); + + it('caps a tool result with a notice saying how to narrow the query', async () => { + const server = new NodeI3xMcpServer(new InProcessConnectorBinding(makeDeps()), { + maxResultChars: 300, + }); + const [clientSide, serverSide] = InMemoryTransport.createLinkedPair(); + // The server object is private: connect it to the in-memory transport for the test. + await ( + server as unknown as { server: { connect(t: unknown): Promise } } + ).server.connect(serverSide); + const client = new Client({ name: 'test', version: '0' }); + await client.connect(clientSide); + + const res = await client.callTool({ + name: 'i3x_explore_plant_hierarchy', + arguments: { maxDepth: 0 }, + }); + const text = (res.content as Array<{ text: string }>)[0]!.text; + expect(text.length).toBeLessThan(600); + expect(text).toMatch(/\[Result truncated: showing the first 300 of \d+ characters/); + expect(text).toMatch(/pass rootElementId/); + await client.close(); + }); +}); + +describe('suggestion cost', () => { + it('does not run fuzzy matching for an over-long query', () => { + const objects = Array.from({ length: 3000 }, (_, i) => ({ + elementId: `instance-${i.toString(16).padStart(16, '0')}`, + displayName: `Asset number ${i} of the plant`, + })); + const started = performance.now(); + expect(rankObjectSuggestions('x'.repeat(5000), objects)).toEqual([]); + expect(performance.now() - started).toBeLessThan(500); + + // Normal queries still match by name. + expect(rankObjectSuggestions('Asset number 42 of the plant', objects)[0]).toEqual({ + elementId: objects[42]!.elementId, + displayName: 'Asset number 42 of the plant', + }); + }); +}); + +describe('history not supported', () => { + it('reports a component the server cannot read history for, not zero-count stats', async () => { + const deps = makeDeps({}, (ds) => { + const read = ds.readHistory.bind(ds); + ds.readHistory = async (...args: Parameters) => { + // Neither the Object nor its variable keeps history. + if (args[0] === 'ns=2;s=Speed' || args[0] === 'ns=2;s=Motor') + throw new Error('BadHistoryOperationUnsupported'); + return read(...args); + }; + }); + const binding = new InProcessConnectorBinding(deps); + const { Motor, Speed } = await idsByName(binding); + + const res = await handleQueryTelemetry(binding, { + elementIds: [Motor!], + mode: 'history', + aggregateStats: true, + maxDepth: 2, + }); + const report = res.telemetryReports?.[0] as Record; + expect(report.components).toEqual({ + [Speed!]: { error: '501 Not Implemented History read not supported' }, + }); + expect(report.note).toMatch(/No component history could be read/); + }); +}); diff --git a/packages/mcp/tests/contract.test.ts b/packages/mcp/tests/contract.test.ts index 6f3b812..4bb4001 100644 --- a/packages/mcp/tests/contract.test.ts +++ b/packages/mcp/tests/contract.test.ts @@ -1,51 +1,11 @@ // 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 { 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'; - -// 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, - }; -} +import { makeDeps } from './helpers/contract-fixture.js'; describe('MCP client against the real rest-server', () => { let binding: InProcessConnectorBinding; diff --git a/packages/mcp/tests/helpers/contract-fixture.ts b/packages/mcp/tests/helpers/contract-fixture.ts new file mode 100644 index 0000000..3fc66ab --- /dev/null +++ b/packages/mcp/tests/helpers/contract-fixture.ts @@ -0,0 +1,47 @@ +// Shared fixture for the contract tests: a real rest-server stack over the +// rest-server tests' MockDataSource, served in-process through inject(). +import { createI3xStack, nullLogger } from '@node-i3x/core'; +import type { RestServerDeps } from '@node-i3x/rest-server'; +import { MockDataSource } from '../../../rest-server/tests/helpers/mock-data-source.js'; + +// Machine (root) ── Temperature, Reset, Motor ── Speed +export function makeDeps( + extra: Partial = {}, + configure?: (ds: MockDataSource) => void, +): 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, + }, + ]; + configure?.(ds); + return { + ...createI3xStack(ds, nullLogger), + dataSource: ds, + logger: nullLogger, + ...extra, + }; +} diff --git a/packages/mcp/tests/mcp-server.test.ts b/packages/mcp/tests/mcp-server.test.ts index d00a891..a2111bf 100644 --- a/packages/mcp/tests/mcp-server.test.ts +++ b/packages/mcp/tests/mcp-server.test.ts @@ -93,6 +93,15 @@ class MockClientBinding implements I3XClientBinding { return []; } + async getRelatedObjectsBulk(elementIds: string[]) { + return Promise.all( + elementIds.map(async (elementId) => ({ + elementId, + result: await this.getRelatedObjects(elementId), + })), + ); + } + async getObjectValues(elementIds: string[]) { return elementIds.map((elementId) => ({ elementId, @@ -312,6 +321,39 @@ describe('@node-i3x/mcp High-Value Tools', () => { ]); }); + it('i3x_monitor_asset_events summarizes alerts and caps the listed ones', async () => { + const binding = new MockClientBinding(); + const components = Object.fromEntries( + Array.from({ length: 60 }, (_, i) => [ + `State_${i}`, + { + value: i < 55 ? null : 1, + quality: (i < 55 ? 'Bad' : 'Uncertain') as 'Bad' | 'Uncertain', + timestamp: '2026-10-25T10:00:00Z', + }, + ]), + ); + binding.getObjectValues = async () => [ + { + elementId: 'Machine', + result: { + isComposition: true, + value: null, + quality: 'GoodNoData' as const, + timestamp: '2026-10-25T10:00:00Z', + components, + }, + }, + ]; + const res = await handleMonitorEvents(binding, { + elementIds: ['Machine'], + maxDepth: 2, + }); + expect(res.summary).toEqual({ bad: 55, uncertain: 5, readErrors: 0 }); + expect(res.alerts).toHaveLength(50); + expect(res.alertsOmitted).toBe(10); + }); + it('i3x_discover_types_and_schemas lists ObjectTypes and schema field rules', async () => { const res = await handleDiscoverTypes(mockBinding, {}); diff --git a/packages/pseudo-session-connector/src/address-space-subscription.ts b/packages/pseudo-session-connector/src/address-space-subscription.ts index 79d16da..24795dc 100644 --- a/packages/pseudo-session-connector/src/address-space-subscription.ts +++ b/packages/pseudo-session-connector/src/address-space-subscription.ts @@ -1,3 +1,4 @@ +import { qualityOfStatusCode } from '@node-i3x/core'; // ───────────────────────────────────────────────────────────── // @node-i3x/pseudo-session-connector — AddressSpaceMonitoredSubscription // Event-based: UAVariable.on("value_changed") @@ -44,7 +45,7 @@ export class AddressSpaceMonitoredSubscription implements IMonitoredSubscription this._cb( nodeId, dataValue.value?.value, - dataValue.statusCode?.name ?? 'Good', + dataValue.statusCode ? qualityOfStatusCode(dataValue.statusCode.value) : 'Good', dataValue.sourceTimestamp?.toISOString() ?? new Date().toISOString(), ); }; diff --git a/packages/pseudo-session-connector/src/polling-subscription.ts b/packages/pseudo-session-connector/src/polling-subscription.ts index 8e6cf5f..562e6ad 100644 --- a/packages/pseudo-session-connector/src/polling-subscription.ts +++ b/packages/pseudo-session-connector/src/polling-subscription.ts @@ -1,3 +1,4 @@ +import { qualityOfStatusCode } from '@node-i3x/core'; // ───────────────────────────────────────────────────────────── // @node-i3x/pseudo-session-connector — PollingMonitoredSubscription // Polling-based: setInterval + IBasicSession.read() @@ -96,7 +97,7 @@ export class PollingMonitoredSubscription implements IMonitoredSubscription { this._cb( nodeId, val, - dv.statusCode?.name ?? 'Good', + dv.statusCode ? qualityOfStatusCode(dv.statusCode.value) : 'Good', dv.sourceTimestamp?.toISOString() ?? new Date().toISOString(), ); } diff --git a/packages/pseudo-session-connector/src/pseudo-session-adapter.ts b/packages/pseudo-session-connector/src/pseudo-session-adapter.ts index 03ccfdc..6e4bd34 100644 --- a/packages/pseudo-session-connector/src/pseudo-session-adapter.ts +++ b/packages/pseudo-session-connector/src/pseudo-session-adapter.ts @@ -15,6 +15,7 @@ import { type ObjectTypeInfo, type ObjectTypeMemberInfo, qualifiedNameToNsu, + qualityOfStatusCode, type SourceDataValue, type SourceHistoricalValue, type SourceNodeInfo, @@ -469,7 +470,7 @@ export class PseudoSessionDataSourceAdapter implements IDataSourcePort { if (dataValues.length > 0) { return dataValues.map((dv) => ({ value: dv.value?.value ?? null, - quality: dv.statusCode?.isGood() ? 'Good' : (dv.statusCode?.name ?? 'Bad'), + quality: qualityOfStatusCode(dv.statusCode?.value), timestamp: dv.sourceTimestamp?.toISOString() ?? new Date().toISOString(), })); } @@ -487,7 +488,7 @@ export class PseudoSessionDataSourceAdapter implements IDataSourcePort { return [ { value: dv.value?.value ?? null, - quality: dv.statusCode?.isGood() ? 'Good' : (dv.statusCode?.name ?? 'Bad'), + quality: qualityOfStatusCode(dv.statusCode?.value), timestamp: dv.sourceTimestamp?.toISOString() ?? new Date().toISOString(), }, ]; diff --git a/packages/pseudo-session-connector/tests/pseudo-session-adapter.test.ts b/packages/pseudo-session-connector/tests/pseudo-session-adapter.test.ts index d8053d3..cc0ebb5 100644 --- a/packages/pseudo-session-connector/tests/pseudo-session-adapter.test.ts +++ b/packages/pseudo-session-connector/tests/pseudo-session-adapter.test.ts @@ -3,6 +3,7 @@ // ───────────────────────────────────────────────────────────── import { consoleLogger } from '@node-i3x/core'; +import { DataType, StatusCodes, Variant } from 'node-opcua'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { PseudoSessionDataSourceAdapter } from '../src/pseudo-session-adapter.js'; import { @@ -224,6 +225,26 @@ describe('PseudoSessionDataSourceAdapter', () => { expect(typeof result[0].value).toBe('number'); }); + it('readHistory reports an i3X quality, not the OPC UA status name', async () => { + const ns = ctx.addressSpace.getNamespace('http://test.i3x.example.com/')!; + const variable = ns.addVariable({ + browseName: 'NeverInitialized', + dataType: 'Double', + organizedBy: ctx.addressSpace.rootFolder.objects, + }); + variable.setValueFromSource( + new Variant({ dataType: DataType.Double, value: 1 }), + StatusCodes.UncertainInitialValue, + ); + const now = new Date(); + const result = await adapter.readHistory( + variable.nodeId.toString(), + new Date(now.getTime() - 3_600_000), + now, + ); + expect(result[0]?.quality).toBe('Uncertain'); + }); + it('readHistory reads from historian when data exists', async () => { const now = new Date(); const oneHourAgo = new Date(now.getTime() - 3_600_000);