diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01bb27a..da449c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,8 @@ jobs: node-version: 22 - name: Install run: npm install --ignore-scripts + - name: Test + run: npm test - name: Build run: npm run build - name: Lint diff --git a/eslint.config.mjs b/eslint.config.mjs index ad811a0..d73cd4f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -1,3 +1,10 @@ import { config } from '@n8n/node-cli/eslint'; -export default config; +export default [ + ...config, + // Unit tests are never published — `files: ["dist"]` ships only the build + // output, and tsconfig compiles `credentials/` and `nodes/` only. n8n + // Cloud's no-dependencies rule is about the node's shipped runtime, so it + // does not apply here; without this the rule rejects `node:test`. + { ignores: ['test/**'] }, +]; diff --git a/nodes/DataGroutMcp/DataGroutMcpTool.node.ts b/nodes/DataGroutMcp/DataGroutMcpTool.node.ts index 4e2b98f..61adfc3 100644 --- a/nodes/DataGroutMcp/DataGroutMcpTool.node.ts +++ b/nodes/DataGroutMcp/DataGroutMcpTool.node.ts @@ -9,6 +9,19 @@ import type { } from 'n8n-workflow'; import { NodeConnectionTypes, NodeOperationError, OperationalError, sleep } from 'n8n-workflow'; +import type { ToolSummary } from './pure'; +import { + MAX_LISTED_TOOLS, + describeTools, + detachedTaskRef, + errorText, + injectLeanDefaults, + parsePossiblySse, + resolveToolName, + taskRecord, + toOutputJson, +} from './pure'; + // ──────────────────────────────────────────────────────────────────── // Minimal MCP client over Streamable HTTP, built on n8n's own // `helpers.httpRequest` so the node carries no runtime dependencies. @@ -20,18 +33,6 @@ const DEFAULT_TASK_WAIT_MS = 120_000; type Ctx = IExecuteFunctions | ILoadOptionsFunctions; -function parsePossiblySse(raw: unknown): IDataObject { - if (typeof raw === 'object' && raw !== null) return raw as IDataObject; - const text = String(raw); - const dataLines = text - .split('\n') - .filter((l) => l.startsWith('data:')) - .map((l) => l.slice(5).trim()) - .filter(Boolean); - const payload = dataLines.length ? dataLines[dataLines.length - 1] : text; - return JSON.parse(payload) as IDataObject; -} - async function mcpRequest( ctx: Ctx, body: IDataObject, @@ -156,15 +157,6 @@ async function mcpListTools(ctx: Ctx, timeoutMs = DEFAULT_TIMEOUT_MS): Promise { - const block = c as { type?: string; text?: string }; - return block?.type === 'text' && typeof block.text === 'string' - ? block.text - : JSON.stringify(c); - }) - .join('\n') - .trim(); - if (text.length) return text; - } - const serialized = JSON.stringify(result); - return serialized && serialized !== 'undefined' ? serialized : '(no result)'; -} - -/** - * Prefer the tool's structuredContent — it is real JSON a workflow can map - * over and an agent can read. Fall back to the flattened text blocks. - */ -function toOutputJson(result: IDataObject): IDataObject { - const structured = result.structuredContent; - if (structured && typeof structured === 'object' && !Array.isArray(structured)) { - return structured as IDataObject; - } - return { result: formatToolResult(result) }; -} - /** The Arguments field arrives as a JSON string when typed, or an object via expression. */ function parseArguments(ctx: IExecuteFunctions, raw: unknown, itemIndex: number): IDataObject { if (raw === undefined || raw === null || raw === '') return {}; @@ -297,13 +233,10 @@ function parseArguments(ctx: IExecuteFunctions, raw: unknown, itemIndex: number) // gateway, and lets an unmatched name return the catalogue instead of an // error the model cannot act on. -const TOOL_LIST_TTL_MS = 5 * 60 * 1000; -const MAX_LISTED_TOOLS = 50; -const MAX_DESCRIPTION_CHARS = 240; - -type ToolSummary = { name: string; description: string }; const toolListCache = new Map(); +const TOOL_LIST_TTL_MS = 5 * 60 * 1000; + async function cachedTools(ctx: Ctx): Promise { const key = await sessionKey(ctx); const cached = toolListCache.get(key); @@ -318,53 +251,6 @@ async function cachedTools(ctx: Ctx): Promise { return tools; } -/** - * The catalogue handed back to a model that asked for an unknown tool. Carries - * descriptions — several DataGrout tools take another tool's fully-qualified - * name as an argument, which a bare list of names gives no way to discover. - * Bounded so a large server cannot flood the model's context. - */ -function describeTools(tools: ToolSummary[]): IDataObject[] { - return tools.slice(0, MAX_LISTED_TOOLS).map((t) => ({ - name: t.name, - description: - t.description.length > MAX_DESCRIPTION_CHARS - ? `${t.description.slice(0, MAX_DESCRIPTION_CHARS)}…` - : t.description, - })); -} - -const normalizeToolName = (s: string) => s.toLowerCase().replace(/[^a-z0-9]/g, ''); - -/** - * Map a requested name onto a real one: exact, then punctuation-insensitive, - * then an unambiguous partial in EITHER direction. A model may write less - * qualification than the server lists (`discovery.plan` for - * `data-grout@1/discovery.plan@1`) or more (`data-grout@1/discovery_perform@1` - * when the server lists the sanitized `discovery_perform`) — both are seen live. - * Ambiguous matches resolve to nothing, so the caller sees the list rather than - * a silently wrong tool being run. - */ -function resolveToolName(requested: string, available: string[]): string | undefined { - if (available.includes(requested)) return requested; - const target = normalizeToolName(requested); - if (!target) return undefined; - const exact = available.filter((n) => normalizeToolName(n) === target); - if (exact.length === 1) return exact[0]; - if (target.length < 4) return undefined; - const partial = available.filter((n) => { - const candidate = normalizeToolName(n); - return candidate.length >= 4 && (candidate.includes(target) || target.includes(candidate)); - }); - return partial.length === 1 ? partial[0] : undefined; -} - -/** Read the error text out of an MCP result flagged isError. */ -function errorText(result: IDataObject): string { - const content = (result.content as IDataObject[]) ?? []; - return (content.find((c) => c.type === 'text')?.text as string) ?? 'Tool returned an error'; -} - // ──────────────────────────────────────────────────────────────────── export class DataGroutMcpTool implements INodeType { diff --git a/nodes/DataGroutMcp/pure.ts b/nodes/DataGroutMcp/pure.ts new file mode 100644 index 0000000..bb76a1d --- /dev/null +++ b/nodes/DataGroutMcp/pure.ts @@ -0,0 +1,146 @@ +import type { IDataObject } from 'n8n-workflow'; + +// ──────────────────────────────────────────────────────────────────── +// Pure helpers — no n8n runtime, no I/O, no state. Kept in their own +// module so they can be unit-tested directly: the task-envelope shape +// and the tool-name resolver below are both subtle enough to have cost +// live debugging cycles. +// ──────────────────────────────────────────────────────────────────── + +export const MAX_LISTED_TOOLS = 50; +export const MAX_DESCRIPTION_CHARS = 240; + +export type ToolSummary = { name: string; description: string }; + +/** + * Parse an MCP response body that may arrive as JSON or as an SSE stream. + * SSE frames are `data:`-prefixed lines; the LAST data frame carries the + * JSON-RPC response. + */ +export function parsePossiblySse(raw: unknown): IDataObject { + if (typeof raw === 'object' && raw !== null) return raw as IDataObject; + const text = String(raw); + const dataLines = text + .split('\n') + .filter((l) => l.startsWith('data:')) + .map((l) => l.slice(5).trim()) + .filter(Boolean); + const payload = dataLines.length ? dataLines[dataLines.length - 1] : text; + return JSON.parse(payload) as IDataObject; +} + +/** + * The task reference when a DataGrout call DETACHED to a background task, + * else undefined. + */ +export function detachedTaskRef(result: IDataObject): string | undefined { + const sc = (result.structuredContent as IDataObject) ?? {}; + if (sc.status === 'detached' && typeof sc.task_ref === 'string') return sc.task_ref; + return undefined; +} + +/** + * The task record inside a `tasks.wait` response. A direct `tools/call` + * returns it at the TOP of structuredContent; the discovery.perform wrapper + * nests it under `.result` (both live-verified 2026-07-23). + */ +export function taskRecord(structuredContent: IDataObject | undefined): IDataObject { + const sc = structuredContent ?? {}; + if (typeof sc.completed !== 'undefined' || sc.task_ref) return sc; + return (sc.result as IDataObject) ?? {}; +} + +/** + * Add DataGrout's response-shaping defaults for discovery tools, so large + * result sets return a preview plus a server-side reference instead of every + * row. Caller-supplied values always win. Matches canonical tool names + * (`data-grout@1/discovery.plan@1`) and the sanitized form some servers list + * (`discovery_plan`). + */ +export function injectLeanDefaults(toolName: string, args: IDataObject): IDataObject { + if (/(^|\/)discovery[._](plan|guide)(@\d+)?$/.test(toolName)) { + return { lean: true, head: true, ...args }; + } + if (/(^|\/)discovery[._]perform(@\d+)?$/.test(toolName)) { + return { head: true, ...args }; + } + return args; +} + +/** Flatten an MCP tool result into a non-empty string. */ +export function formatToolResult(result: IDataObject): string { + const content = result.content; + if (Array.isArray(content)) { + const text = content + .map((c) => { + const block = c as { type?: string; text?: string }; + return block?.type === 'text' && typeof block.text === 'string' + ? block.text + : JSON.stringify(c); + }) + .join('\n') + .trim(); + if (text.length) return text; + } + const serialized = JSON.stringify(result); + return serialized && serialized !== 'undefined' ? serialized : '(no result)'; +} + +/** + * Prefer the tool's structuredContent — it is real JSON a workflow can map + * over and an agent can read. Fall back to the flattened text blocks. + */ +export function toOutputJson(result: IDataObject): IDataObject { + const structured = result.structuredContent; + if (structured && typeof structured === 'object' && !Array.isArray(structured)) { + return structured as IDataObject; + } + return { result: formatToolResult(result) }; +} + +export const normalizeToolName = (s: string) => s.toLowerCase().replace(/[^a-z0-9]/g, ''); + +/** + * Map a requested name onto a real one: exact, then punctuation-insensitive, + * then an unambiguous partial in EITHER direction. A model may write less + * qualification than the server lists (`discovery.plan` for + * `data-grout@1/discovery.plan@1`) or more (`data-grout@1/discovery_perform@1` + * when the server lists the sanitized `discovery_perform`) — both are seen live. + * Ambiguous matches resolve to nothing, so the caller sees the list rather than + * a silently wrong tool being run. + */ +export function resolveToolName(requested: string, available: string[]): string | undefined { + if (available.includes(requested)) return requested; + const target = normalizeToolName(requested); + if (!target) return undefined; + const exact = available.filter((n) => normalizeToolName(n) === target); + if (exact.length === 1) return exact[0]; + if (target.length < 4) return undefined; + const partial = available.filter((n) => { + const candidate = normalizeToolName(n); + return candidate.length >= 4 && (candidate.includes(target) || target.includes(candidate)); + }); + return partial.length === 1 ? partial[0] : undefined; +} + +/** + * The catalogue handed back to a model that asked for an unknown tool. Carries + * descriptions — several DataGrout tools take another tool's fully-qualified + * name as an argument, which a bare list of names gives no way to discover. + * Bounded so a large server cannot flood the model's context. + */ +export function describeTools(tools: ToolSummary[]): IDataObject[] { + return tools.slice(0, MAX_LISTED_TOOLS).map((t) => ({ + name: t.name, + description: + t.description.length > MAX_DESCRIPTION_CHARS + ? `${t.description.slice(0, MAX_DESCRIPTION_CHARS)}…` + : t.description, + })); +} + +/** Read the error text out of an MCP result flagged isError. */ +export function errorText(result: IDataObject): string { + const content = (result.content as IDataObject[]) ?? []; + return (content.find((c) => c.type === 'text')?.text as string) ?? 'Tool returned an error'; +} diff --git a/package.json b/package.json index 6993dae..ab18641 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,8 @@ "dev": "n8n-node dev", "lint": "n8n-node lint", "lint:fix": "n8n-node lint --fix", - "prepublishOnly": "npm run build && npm run lint" + "prepublishOnly": "npm test && npm run build && npm run lint", + "test": "node --test --experimental-strip-types test/*.test.ts" }, "files": [ "dist" @@ -50,4 +51,4 @@ "peerDependencies": { "n8n-workflow": "*" } -} +} \ No newline at end of file diff --git a/test/pure.test.ts b/test/pure.test.ts new file mode 100644 index 0000000..eab09b5 --- /dev/null +++ b/test/pure.test.ts @@ -0,0 +1,263 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; + +import { + MAX_DESCRIPTION_CHARS, + MAX_LISTED_TOOLS, + describeTools, + detachedTaskRef, + errorText, + formatToolResult, + injectLeanDefaults, + normalizeToolName, + parsePossiblySse, + resolveToolName, + taskRecord, + toOutputJson, +} from '../nodes/DataGroutMcp/pure.ts'; + +describe('parsePossiblySse', () => { + it('passes an already-parsed object through', () => { + const obj = { result: { ok: true } }; + assert.equal(parsePossiblySse(obj), obj); + }); + + it('parses a plain JSON body', () => { + assert.deepEqual(parsePossiblySse('{"result":{"a":1}}'), { result: { a: 1 } }); + }); + + it('parses a single SSE data frame', () => { + assert.deepEqual(parsePossiblySse('event: message\ndata: {"result":{"a":1}}\n\n'), { + result: { a: 1 }, + }); + }); + + it('takes the LAST data frame when several are streamed', () => { + const body = 'data: {"result":{"n":1}}\n\ndata: {"result":{"n":2}}\n\n'; + assert.deepEqual(parsePossiblySse(body), { result: { n: 2 } }); + }); + + it('throws on a non-JSON body rather than returning junk', () => { + assert.throws(() => parsePossiblySse('502 Bad Gateway')); + }); +}); + +describe('detachedTaskRef', () => { + it('returns the ref when the call detached', () => { + assert.equal( + detachedTaskRef({ structuredContent: { status: 'detached', task_ref: 'task_abc' } }), + 'task_abc', + ); + }); + + it('returns undefined for an inline (non-detached) result', () => { + assert.equal(detachedTaskRef({ structuredContent: { status: 'ready' } }), undefined); + }); + + it('returns undefined when structuredContent is absent', () => { + assert.equal(detachedTaskRef({ content: [] }), undefined); + }); + + it('ignores a non-string task_ref', () => { + assert.equal( + detachedTaskRef({ structuredContent: { status: 'detached', task_ref: 42 } }), + undefined, + ); + }); +}); + +// Regression cover for the shape bug found by live-testing 2026-07-23: a direct +// tools/call puts the task record at the TOP of structuredContent, while the +// discovery.perform wrapper nests it under .result. +describe('taskRecord', () => { + it('reads a top-level task record (direct tools/call)', () => { + const sc = { completed: true, status: 'completed', result: { executed: true } }; + assert.deepEqual(taskRecord(sc), sc); + }); + + it('reads a nested task record (discovery.perform wrapper)', () => { + const inner = { completed: false, status: 'working' }; + assert.deepEqual(taskRecord({ result: inner }), inner); + }); + + it('treats a bare task_ref as the record', () => { + const sc = { task_ref: 'task_abc', status: 'working' }; + assert.deepEqual(taskRecord(sc), sc); + }); + + it('returns an empty object for an unrecognised or missing envelope', () => { + assert.deepEqual(taskRecord({}), {}); + assert.deepEqual(taskRecord(undefined), {}); + }); +}); + +describe('injectLeanDefaults', () => { + it('adds lean+head for canonical discovery.plan', () => { + assert.deepEqual(injectLeanDefaults('data-grout@1/discovery.plan@1', { goal: 'x' }), { + lean: true, + head: true, + goal: 'x', + }); + }); + + it('adds lean+head for the sanitized name some servers list', () => { + assert.deepEqual(injectLeanDefaults('discovery_plan', {}), { lean: true, head: true }); + }); + + it('adds only head for discovery.perform', () => { + assert.deepEqual(injectLeanDefaults('data-grout@1/discovery.perform@1', {}), { head: true }); + }); + + it('never overrides a caller-supplied value', () => { + assert.deepEqual(injectLeanDefaults('discovery_plan', { head: false }), { + lean: true, + head: false, + }); + }); + + it('leaves non-discovery tools untouched', () => { + const args = { query: 'SELECT Id FROM Opportunity' }; + assert.equal(injectLeanDefaults('salesforce@1/soql@1', args), args); + assert.equal(injectLeanDefaults('discovery_discover', args), args); + }); +}); + +// The resolver is what stands between a model's free-text guess and the +// gateway. Its contract: resolve confidently, or resolve to nothing so the +// caller gets the catalogue back — never silently run a different tool. +describe('resolveToolName', () => { + const available = [ + 'data-grout@1/discovery.plan@1', + 'data-grout@1/discovery.perform@1', + 'atlassian-jira@1/searchjiraissuesusingjql@1', + 'salesforce@1/soql@1', + ]; + + it('returns an exact match unchanged', () => { + assert.equal(resolveToolName('salesforce@1/soql@1', available), 'salesforce@1/soql@1'); + }); + + it('resolves a less-qualified name the model wrote', () => { + assert.equal(resolveToolName('discovery.plan', available), 'data-grout@1/discovery.plan@1'); + assert.equal(resolveToolName('discovery_plan', available), 'data-grout@1/discovery.plan@1'); + }); + + it('resolves a MORE-qualified name against a sanitized listing', () => { + assert.equal( + resolveToolName('data-grout@1/discovery_perform@1', ['discovery_perform']), + 'discovery_perform', + ); + }); + + it('is punctuation- and case-insensitive', () => { + assert.equal(resolveToolName('SOQL', ['salesforce@1/soql@1']), 'salesforce@1/soql@1'); + }); + + it('refuses an ambiguous partial rather than guessing', () => { + // "discovery" matches both plan and perform → caller gets the catalogue + assert.equal(resolveToolName('discovery', available), undefined); + }); + + it('refuses very short fragments that would over-match', () => { + assert.equal(resolveToolName('so', available), undefined); + }); + + it('returns undefined for an empty or punctuation-only request', () => { + assert.equal(resolveToolName('', available), undefined); + assert.equal(resolveToolName('@@@', available), undefined); + }); + + it('returns undefined when nothing resembles the request', () => { + assert.equal(resolveToolName('quickbooks_invoices', available), undefined); + }); + + it('prefers the exact normalized match over a partial one', () => { + // 'search' exists exactly AND is a substring of the jira tool name + const list = ['search', 'atlassian-jira@1/searchjiraissuesusingjql@1']; + assert.equal(resolveToolName('search', list), 'search'); + }); +}); + +describe('describeTools', () => { + it('truncates long descriptions', () => { + const long = 'x'.repeat(MAX_DESCRIPTION_CHARS + 50); + const [only] = describeTools([{ name: 'a', description: long }]); + assert.equal((only.description as string).length, MAX_DESCRIPTION_CHARS + 1); // + ellipsis + assert.ok((only.description as string).endsWith('…')); + }); + + it('leaves short descriptions intact', () => { + const [only] = describeTools([{ name: 'a', description: 'short' }]); + assert.equal(only.description, 'short'); + }); + + it('caps the catalogue so a large server cannot flood the model', () => { + const many = Array.from({ length: MAX_LISTED_TOOLS + 25 }, (_, i) => ({ + name: `tool_${i}`, + description: '', + })); + assert.equal(describeTools(many).length, MAX_LISTED_TOOLS); + }); +}); + +describe('toOutputJson', () => { + it('prefers structuredContent so workflows can map real fields', () => { + const sc = { total: 3, rows: [1, 2, 3] }; + assert.deepEqual(toOutputJson({ structuredContent: sc, content: [] }), sc); + }); + + it('falls back to flattened text when there is no structuredContent', () => { + const res = { content: [{ type: 'text', text: 'hello' }] }; + assert.deepEqual(toOutputJson(res), { result: 'hello' }); + }); + + it('does not treat an array structuredContent as node JSON', () => { + const res = { structuredContent: [1, 2], content: [{ type: 'text', text: 'hi' }] }; + assert.deepEqual(toOutputJson(res), { result: 'hi' }); + }); +}); + +describe('formatToolResult', () => { + it('joins text blocks', () => { + const res = { + content: [ + { type: 'text', text: 'line one' }, + { type: 'text', text: 'line two' }, + ], + }; + assert.equal(formatToolResult(res), 'line one\nline two'); + }); + + it('serialises non-text blocks', () => { + assert.equal( + formatToolResult({ content: [{ type: 'image', data: 'xyz' }] }), + '{"type":"image","data":"xyz"}', + ); + }); + + it('falls back to the whole result when content is empty', () => { + assert.equal(formatToolResult({ content: [], ok: true }), '{"content":[],"ok":true}'); + }); + + it('never returns an empty string', () => { + assert.notEqual(formatToolResult({}), ''); + }); +}); + +describe('errorText', () => { + it('reads the first text block', () => { + const res = { isError: true, content: [{ type: 'text', text: 'boom' }] }; + assert.equal(errorText(res), 'boom'); + }); + + it('has a fallback when no text block is present', () => { + assert.equal(errorText({ isError: true, content: [] }), 'Tool returned an error'); + assert.equal(errorText({ isError: true }), 'Tool returned an error'); + }); +}); + +describe('normalizeToolName', () => { + it('strips punctuation and case', () => { + assert.equal(normalizeToolName('data-grout@1/Discovery.Plan@1'), 'datagrout1discoveryplan1'); + }); +});