mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
Merge branch 'master' into agent-882-feature-message-queueing
This commit is contained in:
commit
91cb077a87
2
.github/test-metrics/e2e-impact-map.json
vendored
2
.github/test-metrics/e2e-impact-map.json
vendored
File diff suppressed because one or more lines are too long
356
packages/@n8n/expression-runtime/src/bridge/host-functions.ts
Normal file
356
packages/@n8n/expression-runtime/src/bridge/host-functions.ts
Normal file
|
|
@ -0,0 +1,356 @@
|
|||
import type { WorkflowData } from '../types';
|
||||
import type { ErrorSentinel } from '../runtime/lazy-proxy';
|
||||
import { bridgeMessageSchema } from './bridge-messages';
|
||||
|
||||
// ============================================================================
|
||||
// Host-side functions shared by the expression bridges.
|
||||
//
|
||||
// Both IsolatedVmBridge and QuickJsBridge expose the same three host
|
||||
// callbacks to the guest — getValueAtPath, getArrayElement, callHost — plus
|
||||
// the error-sentinel helpers and bundle loader around them. The logic is
|
||||
// engine-agnostic: each bridge wraps these pure functions in its own
|
||||
// callback mechanism (ivm.Callback / vm.newFunction) and marshals the
|
||||
// plain, JSON-shaped return values across its own boundary.
|
||||
// ============================================================================
|
||||
|
||||
/** Check if a value is an error sentinel returned by serializeError. */
|
||||
export function isErrorSentinel(value: unknown): value is ErrorSentinel {
|
||||
return (
|
||||
typeof value === 'object' &&
|
||||
value !== null &&
|
||||
(value as Record<string, unknown>).__isError === true
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize an error into a transferable metadata object.
|
||||
*
|
||||
* Host-side callbacks (getValueAtPath, etc.) catch errors and return this
|
||||
* sentinel instead of letting the error cross the boundary (which strips
|
||||
* custom class identity and properties). The guest-side proxy detects
|
||||
* __isError and throws the sentinel; the host reconstructs a real Error
|
||||
* after it round-trips back (see reconstructError).
|
||||
*/
|
||||
export function serializeError(err: unknown): ErrorSentinel {
|
||||
if (err instanceof Error) {
|
||||
const extra = Object.fromEntries(
|
||||
Object.entries(err).filter(([key]) => key !== 'name' && key !== 'message' && key !== 'stack'),
|
||||
);
|
||||
return {
|
||||
__isError: true,
|
||||
name: err.name,
|
||||
message: err.message,
|
||||
stack: err.stack,
|
||||
extra,
|
||||
};
|
||||
}
|
||||
return { __isError: true, name: 'Error', message: String(err), extra: {} };
|
||||
}
|
||||
|
||||
/**
|
||||
* Reconstruct an error from a serialized error sentinel.
|
||||
*
|
||||
* Restores the name, stack, and custom properties that would otherwise be
|
||||
* lost crossing the guest boundary.
|
||||
*/
|
||||
export function reconstructError(data: ErrorSentinel): Error {
|
||||
const error = new Error(data.message);
|
||||
error.name = data.name || 'Error';
|
||||
if (data.stack) {
|
||||
error.stack = data.stack;
|
||||
}
|
||||
|
||||
// Restore custom properties transferred across the boundary
|
||||
if (data.extra) {
|
||||
Object.assign(error, data.extra);
|
||||
}
|
||||
|
||||
return error;
|
||||
}
|
||||
|
||||
/**
|
||||
* Navigate the data object by path and return metadata or a primitive value.
|
||||
*
|
||||
* Used by createDeepLazyProxy when accessing properties. Returns metadata
|
||||
* markers for arrays and objects, or the primitive value directly.
|
||||
*
|
||||
* Special-case: paths starting with ['$item', index] call data.$item(index)
|
||||
* to get the sub-proxy for that item, then continue navigating the rest;
|
||||
* likewise ['$', nodeName] calls data.$(nodeName).
|
||||
*
|
||||
* Function-typed values are returned as `undefined` — every callable on
|
||||
* the host data surface (`$('Foo').first()`, `$items()`, `$fromAI()`,
|
||||
* `$evaluateExpression()`, `$getPairedItem()`) is wired guest-side via
|
||||
* the typed-RPC dispatcher (`callHost`). No expression form should
|
||||
* reach a function through this path (invariant:
|
||||
* __tests__/host-fn-shadowing.test.ts).
|
||||
*/
|
||||
export function getValueAtPath(data: WorkflowData, pathArr: string[]): unknown {
|
||||
let value: unknown = data;
|
||||
let startIndex = 0;
|
||||
const itemFn = (data as Record<string, unknown>).$item;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$item' && typeof itemFn === 'function') {
|
||||
const itemIndex = parseInt(pathArr[1], 10);
|
||||
if (!isNaN(itemIndex)) {
|
||||
value = (itemFn as (i: number) => unknown)(itemIndex);
|
||||
startIndex = 2;
|
||||
}
|
||||
} else {
|
||||
const dollarFn = (data as Record<string, unknown>).$;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$' && typeof dollarFn === 'function') {
|
||||
value = (dollarFn as (name: string) => unknown)(pathArr[1]);
|
||||
startIndex = 2;
|
||||
}
|
||||
}
|
||||
for (let i = startIndex; i < pathArr.length; i++) {
|
||||
value = (value as Record<string, unknown>)?.[pathArr[i]];
|
||||
if (value === undefined || value === null) {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
|
||||
// Functions are not reachable via the lazy-proxy data path —
|
||||
// every callable on the host data surface routes through the
|
||||
// typed-RPC dispatcher. Return undefined so any residual
|
||||
// access surfaces as missing rather than as a stale metadata
|
||||
// marker the runtime no longer knows how to interpret.
|
||||
if (typeof value === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Handle arrays - always lazy, only transfer length
|
||||
if (Array.isArray(value)) {
|
||||
return {
|
||||
__isArray: true,
|
||||
__length: value.length,
|
||||
__data: null,
|
||||
};
|
||||
}
|
||||
|
||||
// Dates have no enumerable own keys; pass through instead of
|
||||
// marshaling as an empty object.
|
||||
if (value instanceof Date) {
|
||||
return value;
|
||||
}
|
||||
|
||||
// Handle objects - return metadata with keys
|
||||
if (value !== null && typeof value === 'object') {
|
||||
return {
|
||||
__isObject: true,
|
||||
__keys: Object.keys(value),
|
||||
};
|
||||
}
|
||||
|
||||
// Primitive value
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get an array element at an index, navigating to the array by path first
|
||||
* (same `$`/`$item` special-casing as getValueAtPath).
|
||||
*
|
||||
* Used by the array proxy when accessing numeric indices.
|
||||
*/
|
||||
export function getArrayElement(data: WorkflowData, pathArr: string[], index: number): unknown {
|
||||
let arr: unknown = data;
|
||||
let startIndex = 0;
|
||||
const itemFn = (data as Record<string, unknown>).$item;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$item' && typeof itemFn === 'function') {
|
||||
const itemIndex = parseInt(pathArr[1], 10);
|
||||
if (!isNaN(itemIndex)) {
|
||||
arr = (itemFn as (i: number) => unknown)(itemIndex);
|
||||
startIndex = 2;
|
||||
}
|
||||
} else {
|
||||
const dollarFn = (data as Record<string, unknown>).$;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$' && typeof dollarFn === 'function') {
|
||||
arr = (dollarFn as (name: string) => unknown)(pathArr[1]);
|
||||
startIndex = 2;
|
||||
}
|
||||
}
|
||||
for (let i = startIndex; i < pathArr.length; i++) {
|
||||
arr = (arr as Record<string, unknown>)?.[pathArr[i]];
|
||||
if (arr === undefined || arr === null) {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
if (!Array.isArray(arr)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Only genuine array indices are reachable; anything else (e.g.
|
||||
// 'constructor', '__lookupGetter__') would read off the prototype
|
||||
// chain and could leak a host function reference across the boundary.
|
||||
if (!Number.isInteger(index) || index < 0) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const element = arr[index];
|
||||
|
||||
// Functions are never reachable through the data surface — mirror the
|
||||
// guard in getValueAtPath so a host callable can't cross the boundary
|
||||
// (invariant: __tests__/host-fn-shadowing.test.ts).
|
||||
if (typeof element === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Dates have no enumerable own keys; pass through instead of
|
||||
// marshaling as an empty object.
|
||||
if (element instanceof Date) {
|
||||
return element;
|
||||
}
|
||||
|
||||
// If element is object/array, return metadata
|
||||
if (element !== null && typeof element === 'object') {
|
||||
if (Array.isArray(element)) {
|
||||
return {
|
||||
__isArray: true,
|
||||
__length: element.length,
|
||||
__data: null,
|
||||
};
|
||||
}
|
||||
return {
|
||||
__isObject: true,
|
||||
__keys: Object.keys(element),
|
||||
};
|
||||
}
|
||||
|
||||
// Primitive element
|
||||
return element;
|
||||
}
|
||||
|
||||
/**
|
||||
* Host-side dispatcher for the typed-RPC `callHost` channel.
|
||||
*
|
||||
* The guest sends one envelope per typed RPC invocation:
|
||||
* `callHost({ type: 'getNodeFirst', nodeName, branchIndex?, runIndex? })`
|
||||
*
|
||||
* Inputs cross a trust boundary, so the dispatcher parses every envelope
|
||||
* with the host-side zod schema (`bridgeMessageSchema`) before any
|
||||
* dispatch happens. Anything that deviates from the declared shape —
|
||||
* unknown `type`, missing required fields, extra unexpected fields,
|
||||
* wrong field types — fails the parse and throws; the calling bridge
|
||||
* catches and returns an error sentinel to the guest.
|
||||
*
|
||||
* After parsing, `switch (msg.type)` dispatches with a fully narrowed
|
||||
* message type. The operation set is exactly the cases in this switch;
|
||||
* the `type` field selects a static branch in source, not a property
|
||||
* lookup on a runtime object. Each case reads a fixed literal property
|
||||
* name off the host-side data proxy — the guest cannot influence which
|
||||
* property is dereferenced.
|
||||
*
|
||||
* Return-value note: cases must return plain, structured-clone-able,
|
||||
* JSON-shaped data. Each bridge marshals the result across its own
|
||||
* boundary (structured clone for isolated-vm, sentinel-wrapped JSON
|
||||
* round-trip for QuickJS), so engine objects or other non-cloneable
|
||||
* values must not be returned.
|
||||
*/
|
||||
export function dispatchHostCall(rawMsg: unknown, data: WorkflowData): unknown {
|
||||
const msg = bridgeMessageSchema.parse(rawMsg);
|
||||
switch (msg.type) {
|
||||
// The `$('Foo').{first,last,all}` typed RPCs.
|
||||
//
|
||||
// Eliminating `data.$` as a host-callable entirely would require
|
||||
// reaching the `WorkflowDataProxy` internals (e.g.
|
||||
// `getNodeExecutionOrPinnedData`) rather than the public `$()` API;
|
||||
// that's a follow-up.
|
||||
//
|
||||
// `data.$` is a host-wired function (`WorkflowDataProxy`'s `$`). If it
|
||||
// ever isn't, optional chaining short-circuits to `undefined` — the
|
||||
// same observable result the runtime's `E()` handler produces from any
|
||||
// thrown error here.
|
||||
case 'getNodeFirst':
|
||||
return data.$?.(msg.nodeName)?.first?.(msg.branchIndex, msg.runIndex);
|
||||
case 'getNodeLast':
|
||||
return data.$?.(msg.nodeName)?.last?.(msg.branchIndex, msg.runIndex);
|
||||
case 'getNodeAll':
|
||||
return data.$?.(msg.nodeName)?.all?.(msg.branchIndex, msg.runIndex);
|
||||
// The `$input.{first,last,all}` typed RPCs.
|
||||
//
|
||||
// Each reads a fixed literal property name off `data.$input` (the
|
||||
// host's `WorkflowDataProxy` input proxy). The host enforces zero
|
||||
// arguments on these methods — the schemas have no fields besides
|
||||
// `type`, so the guest cannot pass anything that would trigger the
|
||||
// "should have no arguments" error path on the host side.
|
||||
case 'getInputFirst':
|
||||
return data.$input?.first?.();
|
||||
case 'getInputLast':
|
||||
return data.$input?.last?.();
|
||||
case 'getInputAll':
|
||||
return data.$input?.all?.();
|
||||
// `$items(nodeName?, outputIndex?, runIndex?)` — the global accessor
|
||||
// for a node's execution data. Reads the literal `$items` property
|
||||
// off `data` (host-wired by `WorkflowDataProxy`) and forwards the
|
||||
// validated args verbatim. The host applies its own defaults when
|
||||
// fields are `undefined`.
|
||||
case 'getItems':
|
||||
return data.$items?.(msg.nodeName, msg.outputIndex, msg.runIndex);
|
||||
// `$fromAI(name, description?, type?, defaultValue?)` and its
|
||||
// `$fromAi` / `$fromai` aliases. Reads the literal `$fromAI` property
|
||||
// off `data` (host-wired) and forwards the args. The host validates
|
||||
// `name` (required + regex) and applies its own resolution / fallback
|
||||
// logic, so empty / invalid names surface as the host's structured
|
||||
// `ExpressionError` rather than a generic zod parse error.
|
||||
//
|
||||
// Note: `msg.valueType` maps to the host's third positional parameter
|
||||
// (`_type` in `WorkflowDataProxy.handleFromAi`). The bridge protocol
|
||||
// renames it to avoid collision with the `type` discriminator on the
|
||||
// envelope — the host parameter currently goes unused, but if it ever
|
||||
// gains a name (`type`), this mapping should stay explicit.
|
||||
case 'fromAi':
|
||||
return data.$fromAI?.(msg.name, msg.description, msg.valueType, msg.defaultValue);
|
||||
// The `$('Foo').pairedItem(itemIndex?)` / `.itemMatching(...)` /
|
||||
// `.item` cluster. Three separate typed RPCs, each reading exactly one
|
||||
// literal property off the host node proxy.
|
||||
//
|
||||
// The split is load-bearing: the host's `pairedItemMethod` closure
|
||||
// captures which property name the proxy `get` trap saw, and uses
|
||||
// that to pick the right error message (e.g. "Missing item index for
|
||||
// .itemMatching()") and to decide between method-call vs getter
|
||||
// semantics for `.item`. Reading the matching property here lets
|
||||
// those host-side branches fire exactly as they do in the legacy
|
||||
// engine; no in-guest validation needed.
|
||||
case 'getNodePairedItem':
|
||||
return data.$?.(msg.nodeName)?.pairedItem?.(msg.itemIndex);
|
||||
case 'getNodeItemMatching':
|
||||
return data.$?.(msg.nodeName)?.itemMatching?.(msg.itemIndex);
|
||||
case 'getNodeItem':
|
||||
// `.item` is a host getter — accessing it invokes the resolver and
|
||||
// returns the value immediately. Optional chaining only short-
|
||||
// circuits on null/undefined; the getter still fires on access.
|
||||
return data.$?.(msg.nodeName)?.item;
|
||||
// `$evaluateExpression(expression, itemIndex?)`. Forwards the string
|
||||
// to the host's nested-evaluation helper, which re-enters the
|
||||
// expression engine on the inner expression. Under the VM engine this
|
||||
// round-trips through the bridge again on a fresh evaluation cycle,
|
||||
// which is the same shape the legacy engine supports.
|
||||
case 'evaluateExpression':
|
||||
return data.$evaluateExpression?.(msg.expression, msg.itemIndex);
|
||||
// `$getPairedItem(destinationNodeName, incomingSourceData,
|
||||
// initialPairedItem)`. Forwards directly to the host binding, which
|
||||
// walks the paired-item ancestry chain back to the named upstream
|
||||
// node and returns the matching execution item.
|
||||
//
|
||||
// The two trailing host parameters — `usedMethodName` and
|
||||
// `nodeBeforeLast` — are deliberately not part of the wire protocol:
|
||||
// the host's default for `usedMethodName` is already `$getPairedItem`,
|
||||
// and `nodeBeforeLast` is an internal recursion argument the host sets
|
||||
// during traversal.
|
||||
case 'getPairedItem':
|
||||
return data.$getPairedItem?.(
|
||||
msg.destinationNodeName,
|
||||
msg.incomingSourceData,
|
||||
msg.initialPairedItem,
|
||||
);
|
||||
default: {
|
||||
// Unreachable at runtime — zod rejects unknown `type` values
|
||||
// before the switch. The `never` assignment is the compile-time
|
||||
// guard: a new schema added to `bridgeMessageSchema` without a
|
||||
// matching case here becomes a type error.
|
||||
const exhaustive: never = msg;
|
||||
void exhaustive;
|
||||
throw new Error('Unhandled bridge message');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -5,7 +5,14 @@ import type { RuntimeBridge, BridgeConfig, ExecuteOptions, WorkflowData } from '
|
|||
import { DEFAULT_BRIDGE_CONFIG, TimeoutError, MemoryLimitError } from '../types';
|
||||
import type { ErrorSentinel } from '../runtime/lazy-proxy';
|
||||
import { unwrapLuxonValues } from '../runtime/luxon-transfer';
|
||||
import { bridgeMessageSchema, type BridgeMessage } from './bridge-messages';
|
||||
import {
|
||||
dispatchHostCall,
|
||||
getArrayElement,
|
||||
getValueAtPath,
|
||||
isErrorSentinel,
|
||||
reconstructError,
|
||||
serializeError,
|
||||
} from './host-functions';
|
||||
|
||||
// Lazy-loaded isolated-vm — avoids loading the native binary when the barrel
|
||||
// file is statically imported (e.g. for error classes). The native module is
|
||||
|
|
@ -81,39 +88,6 @@ const ERROR_HANDLER_SOURCE = `
|
|||
}
|
||||
`;
|
||||
|
||||
/** Check if a value is an error sentinel returned by serializeError. */
|
||||
function isErrorSentinel(value: unknown): value is ErrorSentinel {
|
||||
return (
|
||||
typeof value === 'object' &&
|
||||
value !== null &&
|
||||
(value as Record<string, unknown>).__isError === true
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize an error into a transferable metadata object.
|
||||
*
|
||||
* Host-side callbacks (getValueAtPath, etc.) catch errors and return this
|
||||
* sentinel instead of letting the error cross the isolate boundary (which
|
||||
* strips custom class identity and properties). The isolate-side proxy
|
||||
* detects __isError and reconstructs a proper Error to throw.
|
||||
*/
|
||||
function serializeError(err: unknown): ErrorSentinel {
|
||||
if (err instanceof Error) {
|
||||
const extra = Object.fromEntries(
|
||||
Object.entries(err).filter(([key]) => key !== 'name' && key !== 'message' && key !== 'stack'),
|
||||
);
|
||||
return {
|
||||
__isError: true,
|
||||
name: err.name,
|
||||
message: err.message,
|
||||
stack: err.stack,
|
||||
extra,
|
||||
};
|
||||
}
|
||||
return { __isError: true, name: 'Error', message: String(err), extra: {} };
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the runtime IIFE bundle by walking up from `__dirname` until
|
||||
* `dist/bundle/runtime.iife.js` is found.
|
||||
|
|
@ -325,81 +299,17 @@ export class IsolatedVmBridge implements RuntimeBridge {
|
|||
/**
|
||||
* Create an ivm.Callback for getting value/metadata at a path.
|
||||
*
|
||||
* Used by createDeepLazyProxy when accessing properties. Returns metadata
|
||||
* markers for arrays and objects, or the primitive value directly.
|
||||
*
|
||||
* Function-typed values are returned as `undefined` — every callable on
|
||||
* the host data surface (`$('Foo').first()`, `$items()`, `$fromAI()`,
|
||||
* `$evaluateExpression()`, `$getPairedItem()`) is wired in-isolate via
|
||||
* the typed-RPC dispatcher (`callHost`). No expression form should
|
||||
* reach a function through this path.
|
||||
* Thin wrapper around the shared getValueAtPath (see host-functions.ts
|
||||
* for navigation and guard semantics); errors are caught and returned
|
||||
* as sentinels instead of crossing the isolate boundary.
|
||||
*
|
||||
* @param data - Current workflow data to use for callback responses
|
||||
* @private
|
||||
*/
|
||||
private createGetValueAtPathRef(data: WorkflowData): ivm.Callback {
|
||||
return new (getIvm().Callback)((path: string[]) => {
|
||||
return new (getIvm().Callback)((pathArr: string[]) => {
|
||||
try {
|
||||
// Navigate to value
|
||||
// Special-case: paths starting with ['$item', index] call data.$item(index)
|
||||
// to get the sub-proxy for that item, then continue navigating the rest.
|
||||
let value: unknown = data;
|
||||
let startIndex = 0;
|
||||
const itemFn = (data as Record<string, unknown>).$item;
|
||||
if (path.length >= 2 && path[0] === '$item' && typeof itemFn === 'function') {
|
||||
const itemIndex = parseInt(path[1], 10);
|
||||
if (!isNaN(itemIndex)) {
|
||||
value = (itemFn as (i: number) => unknown)(itemIndex);
|
||||
startIndex = 2;
|
||||
}
|
||||
} else {
|
||||
const dollarFn = (data as Record<string, unknown>).$;
|
||||
if (path.length >= 2 && path[0] === '$' && typeof dollarFn === 'function') {
|
||||
value = (dollarFn as (name: string) => unknown)(path[1]);
|
||||
startIndex = 2;
|
||||
}
|
||||
}
|
||||
for (let i = startIndex; i < path.length; i++) {
|
||||
value = (value as Record<string, unknown>)?.[path[i]];
|
||||
if (value === undefined || value === null) {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
|
||||
// Functions are not reachable via the lazy-proxy data path —
|
||||
// every callable on the host data surface routes through the
|
||||
// typed-RPC dispatcher. Return undefined so any residual
|
||||
// access surfaces as missing rather than as a stale metadata
|
||||
// marker the runtime no longer knows how to interpret.
|
||||
if (typeof value === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Handle arrays - always lazy, only transfer length
|
||||
if (Array.isArray(value)) {
|
||||
return {
|
||||
__isArray: true,
|
||||
__length: value.length,
|
||||
__data: null,
|
||||
};
|
||||
}
|
||||
|
||||
// Dates have no enumerable own keys; pass through instead of
|
||||
// marshaling as an empty object.
|
||||
if (value instanceof Date) {
|
||||
return value;
|
||||
}
|
||||
|
||||
// Handle objects - return metadata with keys
|
||||
if (value !== null && typeof value === 'object') {
|
||||
return {
|
||||
__isObject: true,
|
||||
__keys: Object.keys(value),
|
||||
};
|
||||
}
|
||||
|
||||
// Primitive value
|
||||
return value;
|
||||
return getValueAtPath(data, pathArr);
|
||||
} catch (err) {
|
||||
return serializeError(err);
|
||||
}
|
||||
|
|
@ -409,81 +319,17 @@ export class IsolatedVmBridge implements RuntimeBridge {
|
|||
/**
|
||||
* Create an ivm.Callback for getting array elements at an index.
|
||||
*
|
||||
* Used by array proxy when accessing numeric indices.
|
||||
* Thin wrapper around the shared getArrayElement (see host-functions.ts
|
||||
* for navigation and guard semantics); errors are caught and returned
|
||||
* as sentinels instead of crossing the isolate boundary.
|
||||
*
|
||||
* @param data - Current workflow data to use for callback responses
|
||||
* @private
|
||||
*/
|
||||
private createGetArrayElementRef(data: WorkflowData): ivm.Callback {
|
||||
return new (getIvm().Callback)((path: string[], index: number) => {
|
||||
return new (getIvm().Callback)((pathArr: string[], index: number) => {
|
||||
try {
|
||||
// Navigate to array
|
||||
// Special-case: paths starting with ['$item', index] call data.$item(index)
|
||||
let arr: unknown = data;
|
||||
let startIndex = 0;
|
||||
const itemFn = (data as Record<string, unknown>).$item;
|
||||
if (path.length >= 2 && path[0] === '$item' && typeof itemFn === 'function') {
|
||||
const itemIndex = parseInt(path[1], 10);
|
||||
if (!isNaN(itemIndex)) {
|
||||
arr = (itemFn as (i: number) => unknown)(itemIndex);
|
||||
startIndex = 2;
|
||||
}
|
||||
} else {
|
||||
const dollarFn = (data as Record<string, unknown>).$;
|
||||
if (path.length >= 2 && path[0] === '$' && typeof dollarFn === 'function') {
|
||||
arr = (dollarFn as (name: string) => unknown)(path[1]);
|
||||
startIndex = 2;
|
||||
}
|
||||
}
|
||||
for (let i = startIndex; i < path.length; i++) {
|
||||
arr = (arr as Record<string, unknown>)?.[path[i]];
|
||||
if (arr === undefined || arr === null) {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
if (!Array.isArray(arr)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Only genuine array indices are reachable; anything else (e.g.
|
||||
// 'constructor', '__lookupGetter__') would read off the prototype
|
||||
// chain and could leak a host function reference across the boundary.
|
||||
if (!Number.isInteger(index) || index < 0) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const element = arr[index];
|
||||
|
||||
// Functions are never reachable through the data surface — mirror the
|
||||
// guard in getValueAtPath so a host callable can't cross the boundary.
|
||||
if (typeof element === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Dates have no enumerable own keys; pass through instead of
|
||||
// marshaling as an empty object.
|
||||
if (element instanceof Date) {
|
||||
return element;
|
||||
}
|
||||
|
||||
// If element is object/array, return metadata
|
||||
if (element !== null && typeof element === 'object') {
|
||||
if (Array.isArray(element)) {
|
||||
return {
|
||||
__isArray: true,
|
||||
__length: element.length,
|
||||
__data: null,
|
||||
};
|
||||
}
|
||||
return {
|
||||
__isObject: true,
|
||||
__keys: Object.keys(element),
|
||||
};
|
||||
}
|
||||
|
||||
// Primitive element
|
||||
return element;
|
||||
return getArrayElement(data, pathArr, index);
|
||||
} catch (err) {
|
||||
return serializeError(err);
|
||||
}
|
||||
|
|
@ -491,28 +337,18 @@ export class IsolatedVmBridge implements RuntimeBridge {
|
|||
}
|
||||
|
||||
/**
|
||||
* Create the single typed-RPC dispatcher.
|
||||
* Create the ivm.Callback for the typed-RPC `callHost` channel.
|
||||
*
|
||||
* The isolate sends one envelope per typed RPC invocation:
|
||||
* `callHost({ type: 'getNodeFirst', nodeName, branchIndex?, runIndex? })`
|
||||
* Thin wrapper around the shared dispatchHostCall (see host-functions.ts
|
||||
* for envelope validation and per-message rationale); errors — including
|
||||
* zod parse failures — are caught and returned as sentinels instead of
|
||||
* crossing the isolate boundary.
|
||||
*
|
||||
* Inputs cross a trust boundary, so the dispatcher parses every envelope
|
||||
* with the host-side zod schema (`bridgeMessageSchema`) before any
|
||||
* dispatch happens. Anything that deviates from the declared shape —
|
||||
* unknown `type`, missing required fields, extra unexpected fields,
|
||||
* wrong field types — fails the parse and an error sentinel is returned
|
||||
* to the caller.
|
||||
*
|
||||
* After parsing, `switch (msg.type)` dispatches to a private handler with
|
||||
* a fully narrowed message type. The operation set is exactly the cases
|
||||
* in this switch; the `type` field selects a static branch in source,
|
||||
* not a property lookup on a runtime object.
|
||||
*
|
||||
* Return-value note: handlers must return plain, structured-clone-able
|
||||
* data. Results cross into the isolate through an ivm.Callback, which copies
|
||||
* them via the structured-clone algorithm — return JSON-shaped values, not
|
||||
* isolated-vm objects (`Reference`/`ExternalCopy`) or other non-cloneable
|
||||
* values.
|
||||
* Return-value note: the dispatcher returns plain, structured-clone-able
|
||||
* data. Results cross into the isolate through an ivm.Callback, which
|
||||
* copies them via the structured-clone algorithm — JSON-shaped values,
|
||||
* not isolated-vm objects (`Reference`/`ExternalCopy`) or other
|
||||
* non-cloneable values.
|
||||
*
|
||||
* @param data - Current workflow data
|
||||
* @private
|
||||
|
|
@ -520,230 +356,13 @@ export class IsolatedVmBridge implements RuntimeBridge {
|
|||
private createCallHostRef(data: WorkflowData): ivm.Callback {
|
||||
return new (getIvm().Callback)((rawMsg: unknown) => {
|
||||
try {
|
||||
const msg = bridgeMessageSchema.parse(rawMsg);
|
||||
switch (msg.type) {
|
||||
case 'getNodeFirst':
|
||||
return this.handleGetNodeFirst(msg, data);
|
||||
case 'getNodeLast':
|
||||
return this.handleGetNodeLast(msg, data);
|
||||
case 'getNodeAll':
|
||||
return this.handleGetNodeAll(msg, data);
|
||||
case 'getInputFirst':
|
||||
return this.handleGetInputFirst(data);
|
||||
case 'getInputLast':
|
||||
return this.handleGetInputLast(data);
|
||||
case 'getInputAll':
|
||||
return this.handleGetInputAll(data);
|
||||
case 'getItems':
|
||||
return this.handleGetItems(msg, data);
|
||||
case 'fromAi':
|
||||
return this.handleFromAi(msg, data);
|
||||
case 'getNodePairedItem':
|
||||
return this.handleGetNodePairedItem(msg, data);
|
||||
case 'getNodeItemMatching':
|
||||
return this.handleGetNodeItemMatching(msg, data);
|
||||
case 'getNodeItem':
|
||||
return this.handleGetNodeItem(msg, data);
|
||||
case 'evaluateExpression':
|
||||
return this.handleEvaluateExpression(msg, data);
|
||||
case 'getPairedItem':
|
||||
return this.handleGetPairedItem(msg, data);
|
||||
default: {
|
||||
// Unreachable at runtime — zod rejects unknown `type` values
|
||||
// before the switch. The `never` assignment is the compile-time
|
||||
// guard: a new schema added to `bridgeMessageSchema` without a
|
||||
// matching case here becomes a type error.
|
||||
const exhaustive: never = msg;
|
||||
void exhaustive;
|
||||
throw new Error('Unhandled bridge message');
|
||||
}
|
||||
}
|
||||
return dispatchHostCall(rawMsg, data);
|
||||
} catch (err) {
|
||||
return serializeError(err);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Handlers for the `$('Foo').{first,last,all}` typed RPCs.
|
||||
*
|
||||
* Each handler reads a fixed literal property name off the host-side node
|
||||
* proxy — the isolate cannot influence which property is dereferenced.
|
||||
* Eliminating `data.$` as a host-callable entirely would require reaching
|
||||
* the `WorkflowDataProxy` internals (e.g. `getNodeExecutionOrPinnedData`)
|
||||
* rather than the public `$()` API; that's a follow-up.
|
||||
*
|
||||
* `data.$` is a host-wired function (`WorkflowDataProxy`'s `$`). If it
|
||||
* ever isn't, optional chaining short-circuits to `undefined` — the same
|
||||
* observable result the runtime's `E()` handler produces from any thrown
|
||||
* error here.
|
||||
*
|
||||
* @private
|
||||
*/
|
||||
private handleGetNodeFirst(
|
||||
msg: Extract<BridgeMessage, { type: 'getNodeFirst' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$?.(msg.nodeName)?.first?.(msg.branchIndex, msg.runIndex);
|
||||
}
|
||||
|
||||
private handleGetNodeLast(
|
||||
msg: Extract<BridgeMessage, { type: 'getNodeLast' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$?.(msg.nodeName)?.last?.(msg.branchIndex, msg.runIndex);
|
||||
}
|
||||
|
||||
private handleGetNodeAll(
|
||||
msg: Extract<BridgeMessage, { type: 'getNodeAll' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$?.(msg.nodeName)?.all?.(msg.branchIndex, msg.runIndex);
|
||||
}
|
||||
|
||||
/**
|
||||
* Handlers for the `$input.{first,last,all}` typed RPCs.
|
||||
*
|
||||
* Each reads a fixed literal property name off `data.$input` (the host's
|
||||
* `WorkflowDataProxy` input proxy). The host enforces zero arguments on
|
||||
* these methods — the schemas have no fields besides `type`, so the
|
||||
* isolate cannot pass anything that would trigger the "should have no
|
||||
* arguments" error path on the host side.
|
||||
*
|
||||
* @private
|
||||
*/
|
||||
private handleGetInputFirst(data: WorkflowData): unknown {
|
||||
return data.$input?.first?.();
|
||||
}
|
||||
|
||||
private handleGetInputLast(data: WorkflowData): unknown {
|
||||
return data.$input?.last?.();
|
||||
}
|
||||
|
||||
private handleGetInputAll(data: WorkflowData): unknown {
|
||||
return data.$input?.all?.();
|
||||
}
|
||||
|
||||
/**
|
||||
* Handler for `$items(nodeName?, outputIndex?, runIndex?)` — the
|
||||
* global accessor for a node's execution data. Reads the literal
|
||||
* `$items` property off `data` (host-wired by `WorkflowDataProxy`)
|
||||
* and forwards the validated args verbatim. The host applies its own
|
||||
* defaults when fields are `undefined`.
|
||||
*
|
||||
* @private
|
||||
*/
|
||||
private handleGetItems(
|
||||
msg: Extract<BridgeMessage, { type: 'getItems' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$items?.(msg.nodeName, msg.outputIndex, msg.runIndex);
|
||||
}
|
||||
|
||||
/**
|
||||
* Handler for `$fromAI(name, description?, type?, defaultValue?)` and its
|
||||
* `$fromAi` / `$fromai` aliases. Reads the literal `$fromAI` property
|
||||
* off `data` (host-wired) and forwards the args. The host validates
|
||||
* `name` (required + regex) and applies its own resolution / fallback
|
||||
* logic, so empty / invalid names surface as the host's structured
|
||||
* `ExpressionError` rather than a generic zod parse error.
|
||||
*
|
||||
* Note: `msg.valueType` maps to the host's third positional parameter
|
||||
* (`_type` in `WorkflowDataProxy.handleFromAi`). The bridge protocol
|
||||
* renames it to avoid collision with the `type` discriminator on the
|
||||
* envelope — the host parameter currently goes unused, but if it ever
|
||||
* gains a name (`type`), this mapping should stay explicit.
|
||||
*
|
||||
* @private
|
||||
*/
|
||||
private handleFromAi(
|
||||
msg: Extract<BridgeMessage, { type: 'fromAi' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$fromAI?.(msg.name, msg.description, msg.valueType, msg.defaultValue);
|
||||
}
|
||||
|
||||
/**
|
||||
* Handlers for the `$('Foo').pairedItem(itemIndex?)` / `.itemMatching(...)` /
|
||||
* `.item` cluster. Three separate typed RPCs, each reading exactly one
|
||||
* literal property off the host node proxy.
|
||||
*
|
||||
* The split is load-bearing: the host's `pairedItemMethod` closure
|
||||
* captures which property name the proxy `get` trap saw, and uses
|
||||
* that to pick the right error message (e.g. "Missing item index for
|
||||
* .itemMatching()") and to decide between method-call vs getter
|
||||
* semantics for `.item`. Reading the matching property here lets
|
||||
* those host-side branches fire exactly as they do in the legacy
|
||||
* engine; no in-isolate validation needed.
|
||||
*
|
||||
* @private
|
||||
*/
|
||||
private handleGetNodePairedItem(
|
||||
msg: Extract<BridgeMessage, { type: 'getNodePairedItem' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$?.(msg.nodeName)?.pairedItem?.(msg.itemIndex);
|
||||
}
|
||||
|
||||
private handleGetNodeItemMatching(
|
||||
msg: Extract<BridgeMessage, { type: 'getNodeItemMatching' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$?.(msg.nodeName)?.itemMatching?.(msg.itemIndex);
|
||||
}
|
||||
|
||||
private handleGetNodeItem(
|
||||
msg: Extract<BridgeMessage, { type: 'getNodeItem' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
// `.item` is a host getter — accessing it invokes the resolver and
|
||||
// returns the value immediately. Optional chaining only short-
|
||||
// circuits on null/undefined; the getter still fires on access.
|
||||
return data.$?.(msg.nodeName)?.item;
|
||||
}
|
||||
|
||||
/**
|
||||
* Handler for `$evaluateExpression(expression, itemIndex?)`. Forwards
|
||||
* the string to the host's nested-evaluation helper, which re-enters
|
||||
* the expression engine on the inner expression. Under the VM engine
|
||||
* this round-trips through the bridge again as a new evaluation on the
|
||||
* enclosing call's time budget, which is the same shape the legacy
|
||||
* engine supports.
|
||||
*
|
||||
* @private
|
||||
*/
|
||||
private handleEvaluateExpression(
|
||||
msg: Extract<BridgeMessage, { type: 'evaluateExpression' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$evaluateExpression?.(msg.expression, msg.itemIndex);
|
||||
}
|
||||
|
||||
/**
|
||||
* Handler for `$getPairedItem(destinationNodeName, incomingSourceData,
|
||||
* initialPairedItem)`. Forwards directly to the host binding, which
|
||||
* walks the paired-item ancestry chain back to the named upstream node
|
||||
* and returns the matching execution item.
|
||||
*
|
||||
* The two trailing host parameters — `usedMethodName` and
|
||||
* `nodeBeforeLast` — are deliberately not part of the wire protocol:
|
||||
* the host's default for `usedMethodName` is already `$getPairedItem`,
|
||||
* and `nodeBeforeLast` is an internal recursion argument the host sets
|
||||
* during traversal.
|
||||
*
|
||||
* @private
|
||||
*/
|
||||
private handleGetPairedItem(
|
||||
msg: Extract<BridgeMessage, { type: 'getPairedItem' }>,
|
||||
data: WorkflowData,
|
||||
): unknown {
|
||||
return data.$getPairedItem?.(
|
||||
msg.destinationNodeName,
|
||||
msg.incomingSourceData,
|
||||
msg.initialPairedItem,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute JavaScript code in the isolated context.
|
||||
*
|
||||
|
|
@ -842,7 +461,7 @@ try {
|
|||
);
|
||||
|
||||
if (isErrorSentinel(result)) {
|
||||
throw this.reconstructError(result);
|
||||
throw reconstructError(result);
|
||||
}
|
||||
|
||||
this.logger.debug('[IsolatedVmBridge] Expression executed successfully');
|
||||
|
|
@ -889,27 +508,6 @@ try {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reconstruct an error from serialized isolate data.
|
||||
*
|
||||
* Maps error names back to their host-side classes and restores
|
||||
* custom properties that would otherwise be lost crossing the boundary.
|
||||
*/
|
||||
private reconstructError(data: ErrorSentinel): Error {
|
||||
const error = new Error(data.message);
|
||||
error.name = data.name || 'Error';
|
||||
if (data.stack) {
|
||||
error.stack = data.stack;
|
||||
}
|
||||
|
||||
// Restore custom properties transferred via copy: true
|
||||
if (data.extra) {
|
||||
Object.assign(error, data.extra);
|
||||
}
|
||||
|
||||
return error;
|
||||
}
|
||||
|
||||
/**
|
||||
* Dispose of the isolate and free resources.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -13,7 +13,14 @@ import {
|
|||
TRANSFER_OPAQUE_KEY,
|
||||
TRANSFER_TYPE_KEY,
|
||||
} from '../runtime/transfer';
|
||||
import { bridgeMessageSchema } from './bridge-messages';
|
||||
import {
|
||||
dispatchHostCall,
|
||||
getArrayElement,
|
||||
getValueAtPath,
|
||||
isErrorSentinel,
|
||||
reconstructError,
|
||||
serializeError,
|
||||
} from './host-functions';
|
||||
|
||||
// Lazy-loaded quickjs-emscripten — avoids loading WASM when the barrel
|
||||
// file is statically imported (e.g. for error classes). The module is
|
||||
|
|
@ -103,14 +110,6 @@ function isSetSentinel(value: unknown): value is { __isSet: true; __values: unkn
|
|||
);
|
||||
}
|
||||
|
||||
function isErrorSentinel(value: unknown): value is ErrorSentinel {
|
||||
return (
|
||||
typeof value === 'object' &&
|
||||
value !== null &&
|
||||
(value as Record<string, unknown>).__isError === true
|
||||
);
|
||||
}
|
||||
|
||||
function isEscapedObject(
|
||||
value: unknown,
|
||||
): value is { __isEscaped: true; __value: Record<string, unknown> } {
|
||||
|
|
@ -263,29 +262,6 @@ function wrapSpecialValuesForGuest(value: unknown): unknown {
|
|||
}
|
||||
|
||||
/**
|
||||
* Serialize an error into a transferable metadata object.
|
||||
*
|
||||
* Host-side callbacks (getValueAtPath, etc.) catch errors and return this
|
||||
* sentinel instead of letting the error cross the boundary (which strips
|
||||
* custom class identity and properties). The in-context proxy detects
|
||||
* __isError and throws the sentinel; the host reconstructs a real Error
|
||||
* after it round-trips back (see execute() / reconstructError).
|
||||
*/
|
||||
function serializeError(err: unknown): ErrorSentinel {
|
||||
if (err instanceof Error) {
|
||||
const extra = Object.fromEntries(
|
||||
Object.entries(err).filter(([key]) => key !== 'name' && key !== 'message' && key !== 'stack'),
|
||||
);
|
||||
return {
|
||||
__isError: true,
|
||||
name: err.name,
|
||||
message: err.message,
|
||||
stack: err.stack,
|
||||
extra,
|
||||
};
|
||||
}
|
||||
return { __isError: true, name: 'Error', message: String(err), extra: {} };
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the runtime IIFE bundle by walking up from `__dirname` until
|
||||
|
|
@ -309,6 +285,7 @@ function loadRuntimeBundle(): string {
|
|||
}
|
||||
|
||||
/**
|
||||
|
||||
* Convert a host JavaScript value to a JSON string suitable for round-tripping
|
||||
* into QuickJS via evalCode. Handles undefined (not valid JSON) by returning
|
||||
* the string "undefined".
|
||||
|
|
@ -330,182 +307,6 @@ function hostValueToJson(value: unknown): string {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Navigate data object by path and return metadata or primitive value.
|
||||
* Mirrors the IsolatedVmBridge getValueAtPath callback so QuickJS
|
||||
* matches isolated-vm semantics, including special-cased $/$item navigation.
|
||||
*/
|
||||
function getValueAtPath(data: Record<string, unknown>, pathArr: string[]): unknown {
|
||||
let value: unknown = data;
|
||||
let startIndex = 0;
|
||||
const itemFn = (data as Record<string, unknown>).$item;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$item' && typeof itemFn === 'function') {
|
||||
const itemIndex = parseInt(pathArr[1], 10);
|
||||
if (!isNaN(itemIndex)) {
|
||||
value = (itemFn as (i: number) => unknown)(itemIndex);
|
||||
startIndex = 2;
|
||||
}
|
||||
} else {
|
||||
const dollarFn = (data as Record<string, unknown>).$;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$' && typeof dollarFn === 'function') {
|
||||
value = (dollarFn as (name: string) => unknown)(pathArr[1]);
|
||||
startIndex = 2;
|
||||
}
|
||||
}
|
||||
for (let i = startIndex; i < pathArr.length; i++) {
|
||||
value = (value as Record<string, unknown>)?.[pathArr[i]];
|
||||
if (value === undefined || value === null) {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
|
||||
// Functions must never cross the boundary — resolve them as undefined,
|
||||
// matching IsolatedVmBridge (invariant: __tests__/host-fn-shadowing.test.ts).
|
||||
if (typeof value === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (Array.isArray(value)) {
|
||||
return {
|
||||
__isArray: true,
|
||||
__length: value.length,
|
||||
__data: null,
|
||||
};
|
||||
}
|
||||
|
||||
// Dates have no enumerable own keys; pass through instead of
|
||||
// marshaling as an empty object.
|
||||
if (value instanceof Date) {
|
||||
return value;
|
||||
}
|
||||
|
||||
if (value !== null && typeof value === 'object') {
|
||||
return {
|
||||
__isObject: true,
|
||||
__keys: Object.keys(value),
|
||||
};
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
|
||||
function getArrayElement(data: Record<string, unknown>, pathArr: string[], index: number): unknown {
|
||||
let arr: unknown = data;
|
||||
let startIndex = 0;
|
||||
const itemFn = (data as Record<string, unknown>).$item;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$item' && typeof itemFn === 'function') {
|
||||
const itemIndex = parseInt(pathArr[1], 10);
|
||||
if (!isNaN(itemIndex)) {
|
||||
arr = (itemFn as (i: number) => unknown)(itemIndex);
|
||||
startIndex = 2;
|
||||
}
|
||||
} else {
|
||||
const dollarFn = (data as Record<string, unknown>).$;
|
||||
if (pathArr.length >= 2 && pathArr[0] === '$' && typeof dollarFn === 'function') {
|
||||
arr = (dollarFn as (name: string) => unknown)(pathArr[1]);
|
||||
startIndex = 2;
|
||||
}
|
||||
}
|
||||
for (let i = startIndex; i < pathArr.length; i++) {
|
||||
arr = (arr as Record<string, unknown>)?.[pathArr[i]];
|
||||
if (arr === undefined || arr === null) {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
if (!Array.isArray(arr)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Reject non-integer / negative indices so a crafted "index" can't read off
|
||||
// the prototype chain. Mirrors IsolatedVmBridge.getArrayElement.
|
||||
if (!Number.isInteger(index) || index < 0) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const element = arr[index];
|
||||
|
||||
// Functions must never cross the boundary — resolve as undefined, matching
|
||||
// getValueAtPath and IsolatedVmBridge (invariant: host-fn-shadowing.test.ts).
|
||||
if (typeof element === 'function') {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// Dates have no enumerable own keys; pass through instead of
|
||||
// marshaling as an empty object.
|
||||
if (element instanceof Date) {
|
||||
return element;
|
||||
}
|
||||
|
||||
if (element !== null && typeof element === 'object') {
|
||||
if (Array.isArray(element)) {
|
||||
return {
|
||||
__isArray: true,
|
||||
__length: element.length,
|
||||
__data: null,
|
||||
};
|
||||
}
|
||||
return {
|
||||
__isObject: true,
|
||||
__keys: Object.keys(element),
|
||||
};
|
||||
}
|
||||
|
||||
return element;
|
||||
}
|
||||
|
||||
/**
|
||||
* Host-side dispatcher for the typed-RPC `callHost` channel.
|
||||
*
|
||||
* Mirrors IsolatedVmBridge's dispatcher — see isolated-vm-bridge.ts for the
|
||||
* per-message rationale. The two copies are kept in sync at compile time:
|
||||
* the `never` check in the default case fails to compile when a new schema
|
||||
* lands in bridge-messages.ts without a matching case here.
|
||||
*/
|
||||
function dispatchHostCall(rawMsg: unknown, data: WorkflowData): unknown {
|
||||
const msg = bridgeMessageSchema.parse(rawMsg);
|
||||
switch (msg.type) {
|
||||
case 'getNodeFirst':
|
||||
return data.$?.(msg.nodeName)?.first?.(msg.branchIndex, msg.runIndex);
|
||||
case 'getNodeLast':
|
||||
return data.$?.(msg.nodeName)?.last?.(msg.branchIndex, msg.runIndex);
|
||||
case 'getNodeAll':
|
||||
return data.$?.(msg.nodeName)?.all?.(msg.branchIndex, msg.runIndex);
|
||||
case 'getInputFirst':
|
||||
return data.$input?.first?.();
|
||||
case 'getInputLast':
|
||||
return data.$input?.last?.();
|
||||
case 'getInputAll':
|
||||
return data.$input?.all?.();
|
||||
case 'getItems':
|
||||
return data.$items?.(msg.nodeName, msg.outputIndex, msg.runIndex);
|
||||
case 'fromAi':
|
||||
return data.$fromAI?.(msg.name, msg.description, msg.valueType, msg.defaultValue);
|
||||
case 'getNodePairedItem':
|
||||
return data.$?.(msg.nodeName)?.pairedItem?.(msg.itemIndex);
|
||||
case 'getNodeItemMatching':
|
||||
return data.$?.(msg.nodeName)?.itemMatching?.(msg.itemIndex);
|
||||
case 'getNodeItem':
|
||||
// `.item` is a host getter — accessing it invokes the resolver.
|
||||
return data.$?.(msg.nodeName)?.item;
|
||||
case 'evaluateExpression':
|
||||
return data.$evaluateExpression?.(msg.expression, msg.itemIndex);
|
||||
case 'getPairedItem':
|
||||
return data.$getPairedItem?.(
|
||||
msg.destinationNodeName,
|
||||
msg.incomingSourceData,
|
||||
msg.initialPairedItem,
|
||||
);
|
||||
default: {
|
||||
// Unreachable at runtime — zod rejects unknown `type` values before
|
||||
// the switch. The `never` assignment is the compile-time guard.
|
||||
const exhaustive: never = msg;
|
||||
void exhaustive;
|
||||
throw new Error('Unhandled bridge message');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* QuickJsBridge - Runtime bridge using quickjs-emscripten for expression evaluation.
|
||||
*
|
||||
|
|
@ -1357,7 +1158,7 @@ export class QuickJsBridge implements RuntimeBridge {
|
|||
const result = unwrapSentinels(rawResult);
|
||||
|
||||
if (isErrorSentinel(result)) {
|
||||
throw this.reconstructError(result);
|
||||
throw reconstructError(result);
|
||||
}
|
||||
|
||||
this.logger.debug('[QuickJsBridge] Expression executed successfully');
|
||||
|
|
@ -1406,18 +1207,6 @@ export class QuickJsBridge implements RuntimeBridge {
|
|||
);
|
||||
}
|
||||
|
||||
private reconstructError(data: ErrorSentinel): Error {
|
||||
const error = new Error(data.message);
|
||||
error.name = data.name || 'Error';
|
||||
if (data.stack) {
|
||||
error.stack = data.stack;
|
||||
}
|
||||
if (data.extra) {
|
||||
Object.assign(error, data.extra);
|
||||
}
|
||||
return error;
|
||||
}
|
||||
|
||||
private evalCodeOrThrow(code: string, label: string): unknown {
|
||||
if (!this.vm) throw new Error('Context not initialized');
|
||||
|
||||
|
|
|
|||
|
|
@ -219,7 +219,9 @@ follow its build → publish → assign steps.
|
|||
Decide grouping now, while writing the source: `.group(...)` lives in the code, so
|
||||
it cannot be added after the build. See [Node Groups](#node-groups) for the
|
||||
criteria, and reach a decision either way — groups declared, or this workflow does
|
||||
not warrant them.
|
||||
not warrant them. When the canvas will be over the ceiling and no valid group can hold
|
||||
the remaining nodes, pass `groupingDecision: 'not_warranted'` with a `groupingReason`
|
||||
to `build-workflow`; without groups or that reason the build is refused.
|
||||
7. Before the first `build-workflow` (and again after substantive edits), run
|
||||
SDK validation on the workspace source file via
|
||||
`workspace_execute_command`:
|
||||
|
|
@ -992,9 +994,13 @@ store its own public endpoint.
|
|||
Do not report a build as done until you have made the grouping decision described in
|
||||
[Node Groups](#node-groups) and checked what the build did with it. A dropped-group warning
|
||||
names what was invalid — a duplicate name, a member that does not exist, a boundary the rules
|
||||
reject: fix what the warning reports and build again. If the top level is still above
|
||||
{{TOP_LEVEL_ITEM_CEILING_PLACEHOLDER}} items, name each remaining item and why it cannot join
|
||||
a group — if you cannot, group it and build again.
|
||||
reject: fix what the warning reports and build again. A `GROUPING_DECISION_MISSING` error means
|
||||
the build was refused: fix the source, or pass the opt-out with a reason. A
|
||||
`GROUP_DROPPED_OVER_CEILING` error also refuses the build: a declared group was invalid and the
|
||||
canvas is still over the ceiling. Fix the boundary the message names — the opt-out does not
|
||||
apply. If the top level is still above
|
||||
{{TOP_LEVEL_ITEM_CEILING_PLACEHOLDER}} items with groups in place, name each remaining item and
|
||||
why it cannot join a group.
|
||||
|
||||
For a successful build, finish with one concise sentence naming the workflow and
|
||||
what changed. Include the workflow ID when it is available. If setup is
|
||||
|
|
|
|||
|
|
@ -103,6 +103,14 @@ vi.mock('../generate-simulation-fixtures.service', async (importOriginal) => ({
|
|||
type BuildToolOutput = {
|
||||
success: boolean;
|
||||
filePath: string;
|
||||
grouping?: {
|
||||
topLevelItemCount: number;
|
||||
ceiling: number;
|
||||
groupCount: number;
|
||||
droppedGroupCount: number;
|
||||
decision: string;
|
||||
reason?: string;
|
||||
};
|
||||
sourceHash?: string;
|
||||
workflowId?: string;
|
||||
workflowName?: string;
|
||||
|
|
@ -565,6 +573,269 @@ describe('createBuildWorkflowTool', () => {
|
|||
);
|
||||
});
|
||||
|
||||
describe('grouping decision check', () => {
|
||||
const wideWorkflow = (nodeGroups?: Array<{ id: string; name: string; nodeIds: string[] }>) => ({
|
||||
name: 'Wide workflow',
|
||||
nodes: Array.from({ length: 8 }, (_, i) => ({
|
||||
id: `node-${i}`,
|
||||
name: `Step ${i}`,
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 1,
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {},
|
||||
})),
|
||||
connections: {},
|
||||
...(nodeGroups ? { nodeGroups } : {}),
|
||||
});
|
||||
const compileTo = (workflow: ReturnType<typeof wideWorkflow>) =>
|
||||
vi.mocked(compileWorkflowSource).mockResolvedValueOnce({
|
||||
success: true,
|
||||
workflow,
|
||||
warnings: [],
|
||||
compiler: 'sandbox-tsx',
|
||||
});
|
||||
|
||||
it('refuses a new canvas over the ceiling that declares no group', async () => {
|
||||
const { context, filePath, trackTelemetry } = makeContext({ source: 'src' });
|
||||
compileTo(wideWorkflow());
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.join('\n')).toContain('[GROUPING_DECISION_MISSING]');
|
||||
expect(result.errors?.join('\n')).toContain('Step 0, Step 1');
|
||||
expect(result.remediation?.reason).toBe('workflow_grouping_decision_missing');
|
||||
expect(result.grouping).toMatchObject({ groupCount: 0, decision: 'missing' });
|
||||
expect(context.workflowService.createFromWorkflowJSON).not.toHaveBeenCalled();
|
||||
expect(trackTelemetry).toHaveBeenCalledWith(
|
||||
'instance_ai_workflow_source_build',
|
||||
expect.objectContaining({
|
||||
result: 'failure',
|
||||
stage: 'grouping',
|
||||
top_level_item_count: 8,
|
||||
group_count: 0,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('saves the same canvas when the agent opts out with a reason, and records it', async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
compileTo(wideWorkflow());
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
groupingDecision: 'not_warranted',
|
||||
groupingReason: 'every step fans out to its own branch',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.grouping).toMatchObject({
|
||||
topLevelItemCount: 8,
|
||||
groupCount: 0,
|
||||
decision: 'not_warranted',
|
||||
reason: 'every step fans out to its own branch',
|
||||
});
|
||||
expect(result.warnings?.join('\n')).toContain(
|
||||
'(accepted: every step fans out to its own branch)',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects the opt-out without a reason before compiling', async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
groupingDecision: 'not_warranted',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.remediation?.reason).toBe('grouping_reason_missing');
|
||||
expect(compileWorkflowSource).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('saves a canvas over the ceiling when a valid group survived', async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
compileTo(
|
||||
wideWorkflow([{ id: 'g1', name: 'Stage', nodeIds: ['node-0', 'node-1', 'node-2'] }]),
|
||||
);
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.grouping).toMatchObject({
|
||||
topLevelItemCount: 6,
|
||||
groupCount: 1,
|
||||
decision: 'grouped',
|
||||
});
|
||||
expect(result.warnings).toBeUndefined();
|
||||
});
|
||||
|
||||
it('refuses when every declared group was dropped, even with the opt-out', async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
compileTo(wideWorkflow([{ id: 'g1', name: 'Stage', nodeIds: ['missing-a', 'missing-b'] }]));
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
groupingDecision: 'not_warranted',
|
||||
groupingReason: 'does not matter here',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.join('\n')).toContain('[GROUP_DROPPED_OVER_CEILING]');
|
||||
expect(result.errors?.join('\n')).toContain('Stage');
|
||||
// The error carries the drop reason; the matching warning is not repeated.
|
||||
expect(result.warnings?.join('\n') ?? '').not.toContain('[NODE_GROUP_DROPPED]');
|
||||
expect(result.remediation?.reason).toBe('workflow_group_dropped_over_ceiling');
|
||||
expect(result.grouping).toMatchObject({ groupCount: 0, decision: 'grouped' });
|
||||
expect(context.workflowService.createFromWorkflowJSON).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses when one declared group was dropped and the canvas is still over the ceiling', async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
const wide = wideWorkflow([
|
||||
{ id: 'g1', name: 'Kept', nodeIds: ['node-0', 'node-1'] },
|
||||
{ id: 'g2', name: 'Broken', nodeIds: ['missing-a', 'missing-b'] },
|
||||
]);
|
||||
// 10 nodes: 2 in the kept group + 8 loose = 9 boxes, still over the ceiling.
|
||||
wide.nodes.push(
|
||||
...[8, 9].map((i) => ({
|
||||
id: `node-${i}`,
|
||||
name: `Step ${i}`,
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 1,
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {},
|
||||
})),
|
||||
);
|
||||
compileTo(wide);
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.join('\n')).toContain('[GROUP_DROPPED_OVER_CEILING]');
|
||||
expect(result.errors?.join('\n')).toContain('Broken');
|
||||
expect(result.remediation?.reason).toBe('workflow_group_dropped_over_ceiling');
|
||||
expect(result.grouping).toMatchObject({
|
||||
topLevelItemCount: 9,
|
||||
groupCount: 1,
|
||||
droppedGroupCount: 1,
|
||||
decision: 'grouped',
|
||||
});
|
||||
expect(context.workflowService.createFromWorkflowJSON).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('leaves a canvas at the ceiling alone', async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
const atCeiling = wideWorkflow();
|
||||
atCeiling.nodes = atCeiling.nodes.slice(0, 7);
|
||||
compileTo(atCeiling);
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.grouping).toMatchObject({ topLevelItemCount: 7, decision: 'under_ceiling' });
|
||||
expect(result.warnings).toBeUndefined();
|
||||
});
|
||||
|
||||
describe('on an existing workflow', () => {
|
||||
const snapshotWith = (count: number) => ({
|
||||
name: 'Target workflow',
|
||||
nodes: Array.from({ length: count }, (_, i) => ({
|
||||
id: `old-${i}`,
|
||||
name: `Old ${i}`,
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 1,
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {},
|
||||
})),
|
||||
connections: {},
|
||||
});
|
||||
|
||||
it("saves the user's workflow that was already over the ceiling, with a warning only", async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
vi.mocked(context.workflowService.getAsWorkflowJSON).mockResolvedValueOnce(
|
||||
snapshotWith(9) as never,
|
||||
);
|
||||
compileTo(wideWorkflow());
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
workflowId: 'wf-user',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.warnings?.join('\n')).toContain('[TOP_LEVEL_ITEMS_OVER_CEILING]');
|
||||
expect(result.warnings?.join('\n')).not.toContain('GROUPING_DECISION_MISSING');
|
||||
expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("refuses a dropped group even on the user's workflow that was already over the ceiling", async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
vi.mocked(context.workflowService.getAsWorkflowJSON).mockResolvedValueOnce(
|
||||
snapshotWith(9) as never,
|
||||
);
|
||||
compileTo(
|
||||
wideWorkflow([{ id: 'g1', name: 'Broken', nodeIds: ['missing-a', 'missing-b'] }]),
|
||||
);
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
workflowId: 'wf-user',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.join('\n')).toContain('[GROUP_DROPPED_OVER_CEILING]');
|
||||
expect(result.errors?.join('\n')).toContain('Broken');
|
||||
expect(context.workflowService.updateFromWorkflowJSON).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses when the edit pushes a workflow that was under the ceiling over it with no groups', async () => {
|
||||
const { context, filePath } = makeContext({ source: 'src' });
|
||||
vi.mocked(context.workflowService.getAsWorkflowJSON).mockResolvedValueOnce(
|
||||
snapshotWith(6) as never,
|
||||
);
|
||||
compileTo(wideWorkflow());
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
workflowId: 'wf-user',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.join('\n')).toContain('[GROUPING_DECISION_MISSING]');
|
||||
expect(context.workflowService.updateFromWorkflowJSON).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses on a workflow this run created, even when it was already over the ceiling', async () => {
|
||||
const { context, filePath } = makeContext({
|
||||
source: 'src',
|
||||
overrides: { aiCreatedWorkflowIds: new Set(['wf-created']) },
|
||||
});
|
||||
vi.mocked(context.workflowService.getAsWorkflowJSON).mockResolvedValueOnce(
|
||||
snapshotWith(9) as never,
|
||||
);
|
||||
compileTo(wideWorkflow());
|
||||
|
||||
const result = await executeTool<BuildToolOutput>(createBuildWorkflowTool(context), {
|
||||
filePath,
|
||||
workflowId: 'wf-created',
|
||||
});
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.errors?.join('\n')).toContain('[GROUPING_DECISION_MISSING]');
|
||||
expect(context.workflowService.updateFromWorkflowJSON).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('falls back to the post-build-flow handoff for a triggerless one-off build', async () => {
|
||||
const source = 'workflow source from workspace';
|
||||
const { context, filePath } = makeContext({ source });
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@ import type { WorkflowJSON } from '@n8n/workflow-sdk';
|
|||
import type { IConnections } from 'n8n-workflow';
|
||||
|
||||
import {
|
||||
groupingDecisionBlocker,
|
||||
partitionWarnings,
|
||||
summarizeWorkflowTopLevelItems,
|
||||
topLevelItemsWarning,
|
||||
type ValidationWarning,
|
||||
} from '../workflow-validation-warnings';
|
||||
|
|
@ -120,3 +122,112 @@ describe('topLevelItemsWarning', () => {
|
|||
expect(topLevelItemsWarning(withSticky)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('groupingDecisionBlocker', () => {
|
||||
const node = (name: string) => ({
|
||||
id: name,
|
||||
name,
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [0, 0] as [number, number],
|
||||
});
|
||||
const names = (count: number) => Array.from({ length: count }, (_, i) => `n${i}`);
|
||||
const workflow = (count: number, extra: Partial<WorkflowJSON> = {}): WorkflowJSON => ({
|
||||
name: 'wf',
|
||||
nodes: names(count).map(node),
|
||||
connections: {},
|
||||
...extra,
|
||||
});
|
||||
const dropped = [
|
||||
{
|
||||
code: 'NODE_GROUP_DROPPED',
|
||||
severity: 'informational' as const,
|
||||
message: 'Node group "Body" was removed: reason.',
|
||||
},
|
||||
];
|
||||
|
||||
it('says nothing under the ceiling', () => {
|
||||
const summary = summarizeWorkflowTopLevelItems(workflow(7));
|
||||
|
||||
expect(
|
||||
groupingDecisionBlocker({ summary, declaredGroupCount: 0, droppedGroupWarnings: [] }),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('says nothing when a group survived, even over the ceiling', () => {
|
||||
const summary = summarizeWorkflowTopLevelItems(
|
||||
workflow(9, { nodeGroups: [{ id: 'g', name: 'Stage', nodeIds: ['n0', 'n1'] }] }),
|
||||
);
|
||||
|
||||
expect(
|
||||
groupingDecisionBlocker({ summary, declaredGroupCount: 1, droppedGroupWarnings: [] }),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('blocks an over-ceiling canvas with no group declared, naming the groupable nodes', () => {
|
||||
const summary = summarizeWorkflowTopLevelItems(workflow(8));
|
||||
|
||||
const blocker = groupingDecisionBlocker({
|
||||
summary,
|
||||
declaredGroupCount: 0,
|
||||
droppedGroupWarnings: [],
|
||||
});
|
||||
|
||||
expect(blocker?.code).toBe('GROUPING_DECISION_MISSING');
|
||||
expect(blocker?.severity).toBe('warning');
|
||||
expect(blocker?.message).toContain('8 boxes');
|
||||
expect(blocker?.message).toContain('n0, n1');
|
||||
expect(blocker?.message).toContain("groupingDecision: 'not_warranted'");
|
||||
});
|
||||
|
||||
it('lets the explicit opt-out through', () => {
|
||||
const summary = summarizeWorkflowTopLevelItems(workflow(8));
|
||||
|
||||
expect(
|
||||
groupingDecisionBlocker({
|
||||
summary,
|
||||
declaredGroupCount: 0,
|
||||
droppedGroupWarnings: [],
|
||||
groupingDecision: 'not_warranted',
|
||||
}),
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('blocks when every declared group was dropped, and the opt-out does not excuse it', () => {
|
||||
const summary = summarizeWorkflowTopLevelItems(workflow(8));
|
||||
|
||||
const blocker = groupingDecisionBlocker({
|
||||
summary,
|
||||
declaredGroupCount: 1,
|
||||
droppedGroupWarnings: dropped,
|
||||
groupingDecision: 'not_warranted',
|
||||
});
|
||||
|
||||
expect(blocker?.code).toBe('GROUP_DROPPED_OVER_CEILING');
|
||||
expect(blocker?.severity).toBe('warning');
|
||||
expect(blocker?.message).toContain('Node group "Body" was removed: reason.');
|
||||
});
|
||||
|
||||
it('blocks when one declared group was dropped and another survived, while still over the ceiling', () => {
|
||||
const summary = summarizeWorkflowTopLevelItems(
|
||||
workflow(10, { nodeGroups: [{ id: 'g1', name: 'Kept', nodeIds: ['n0', 'n1'] }] }),
|
||||
);
|
||||
|
||||
const blocker = groupingDecisionBlocker({
|
||||
summary,
|
||||
declaredGroupCount: 2,
|
||||
droppedGroupWarnings: dropped,
|
||||
});
|
||||
|
||||
expect(blocker?.code).toBe('GROUP_DROPPED_OVER_CEILING');
|
||||
expect(blocker?.message).toContain('1 of 2 declared node group(s) were removed');
|
||||
});
|
||||
|
||||
it('does not block a dropped group when the canvas is within the ceiling', () => {
|
||||
const summary = summarizeWorkflowTopLevelItems(workflow(6));
|
||||
|
||||
expect(
|
||||
groupingDecisionBlocker({ summary, declaredGroupCount: 1, droppedGroupWarnings: dropped }),
|
||||
).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -55,6 +55,7 @@ import { withDeterministicRouting } from './workflow-build-routing';
|
|||
import {
|
||||
trackWaitGateVerificationPlan,
|
||||
trackWorkflowSourceBuild,
|
||||
type BuildTelemetryStage,
|
||||
} from './workflow-build-telemetry';
|
||||
import {
|
||||
bindSourceFileToExistingWorkflow,
|
||||
|
|
@ -78,8 +79,12 @@ import {
|
|||
import { computeChangedNodeNames, downgradeUnchangedNodeBlockers } from './workflow-node-diff';
|
||||
import { compileWorkflowSource } from './workflow-source-compiler';
|
||||
import {
|
||||
GROUP_DROPPED_OVER_CEILING_CODE,
|
||||
groupingDecisionBlocker,
|
||||
NODE_GROUP_DROPPED_CODE,
|
||||
nodeGroupDroppedWarnings,
|
||||
partitionWarnings,
|
||||
summarizeWorkflowTopLevelItems,
|
||||
topLevelItemsWarning,
|
||||
type ValidationWarning,
|
||||
} from './workflow-validation-warnings';
|
||||
|
|
@ -91,8 +96,10 @@ import type { FolderResolutionFailure, InstanceAiContext, WorkflowFolderRef } fr
|
|||
import { BuildFailureTracker } from '../../workflow-builder/build-failure-tracker';
|
||||
import { createRemediation } from '../../workflow-loop/remediation';
|
||||
import {
|
||||
groupingOutcomeSchema,
|
||||
remediationMetadataSchema,
|
||||
workflowVerificationReadinessSchema,
|
||||
type GroupingOutcome,
|
||||
type WorkflowBuildOutcome,
|
||||
} from '../../workflow-loop/workflow-loop-state';
|
||||
import { writeWorkspaceFile } from '../../workspace/workspace-files';
|
||||
|
|
@ -204,6 +211,20 @@ export const buildWorkflowInputSchema = z
|
|||
'output was read back (see the one-off-operations skill). Omit or pass `reusable` for ' +
|
||||
'anything the user may run again.',
|
||||
),
|
||||
groupingDecision: z
|
||||
.enum(['grouped', 'not_warranted'])
|
||||
.optional()
|
||||
.describe(
|
||||
'Only for a canvas that will exceed the top-level ceiling with no node group: pass `not_warranted` ' +
|
||||
'together with `groupingReason` to say why no valid group can hold the remaining nodes. ' +
|
||||
'Never pass it to skip the grouping decision.',
|
||||
),
|
||||
groupingReason: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe(
|
||||
'Required with `groupingDecision: not_warranted`: why these nodes cannot form a valid group.',
|
||||
),
|
||||
})
|
||||
.strict();
|
||||
|
||||
|
|
@ -412,6 +433,41 @@ interface ValidationFailureArgs {
|
|||
isSupportingWorkflow?: boolean;
|
||||
isAuxiliarySupportingWorkflow?: boolean;
|
||||
withEscalation: (errors: string[]) => string[];
|
||||
stage?: BuildTelemetryStage;
|
||||
grouping?: GroupingOutcome;
|
||||
}
|
||||
|
||||
/**
|
||||
* The grouping decision this build ends with, for the result and telemetry.
|
||||
*
|
||||
* - `grouped`: the agent made groups, even if the save dropped all of them.
|
||||
* - `not_warranted`: the agent made no groups and gave a reason.
|
||||
* - `under_ceiling`: the canvas has `TOP_LEVEL_ITEM_CEILING` boxes or fewer, so
|
||||
* groups are not needed.
|
||||
* - `missing`: the canvas has more than `TOP_LEVEL_ITEM_CEILING` boxes, and the
|
||||
* agent made no groups and gave no reason. It skipped the decision. The build
|
||||
* is refused.
|
||||
*/
|
||||
function resolveGroupingDecision(input: {
|
||||
groupCount: number;
|
||||
overCeiling: boolean;
|
||||
groupingDecision: 'grouped' | 'not_warranted' | undefined;
|
||||
}): GroupingOutcome['decision'] {
|
||||
if (input.groupCount > 0) {
|
||||
return 'grouped';
|
||||
}
|
||||
|
||||
if (input.groupingDecision !== undefined) {
|
||||
return input.groupingDecision;
|
||||
}
|
||||
|
||||
// No groups and no reason given. With more than `TOP_LEVEL_ITEM_CEILING` boxes on
|
||||
// the canvas, the agent had to make groups or give a reason. It did neither.
|
||||
if (input.overCeiling) {
|
||||
return 'missing';
|
||||
}
|
||||
|
||||
return 'under_ceiling';
|
||||
}
|
||||
|
||||
async function handleValidationFailure(args: ValidationFailureArgs) {
|
||||
|
|
@ -433,6 +489,8 @@ async function handleValidationFailure(args: ValidationFailureArgs) {
|
|||
isSupportingWorkflow = false,
|
||||
isAuxiliarySupportingWorkflow = false,
|
||||
withEscalation,
|
||||
stage = 'validation',
|
||||
grouping,
|
||||
} = args;
|
||||
|
||||
const formattedErrors = withEscalation(
|
||||
|
|
@ -451,10 +509,11 @@ async function handleValidationFailure(args: ValidationFailureArgs) {
|
|||
errors: formattedErrors,
|
||||
summary,
|
||||
storeOnRunContext: !isAuxiliarySupportingWorkflow,
|
||||
grouping,
|
||||
});
|
||||
trackWorkflowSourceBuild(context, {
|
||||
result: 'failure',
|
||||
stage: 'validation',
|
||||
stage,
|
||||
binding,
|
||||
targetWorkflowId,
|
||||
isSupportingWorkflow,
|
||||
|
|
@ -462,6 +521,15 @@ async function handleValidationFailure(args: ValidationFailureArgs) {
|
|||
remediation,
|
||||
errorCount: formattedErrors.length,
|
||||
warningCount: informational.length,
|
||||
...(grouping
|
||||
? {
|
||||
topLevelItemCount: grouping.topLevelItemCount,
|
||||
groupCount: grouping.groupCount,
|
||||
droppedGroupCount: grouping.droppedGroupCount,
|
||||
groupingDecision: grouping.decision,
|
||||
groupingReasonProvided: grouping.reason !== undefined,
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
return {
|
||||
success: false as const,
|
||||
|
|
@ -471,6 +539,7 @@ async function handleValidationFailure(args: ValidationFailureArgs) {
|
|||
errors: formattedErrors,
|
||||
remediation,
|
||||
warnings: combineWarnings(informational.map((w) => formatWarning(w.code, w.message))),
|
||||
...(grouping ? { grouping } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -496,6 +565,7 @@ const buildWorkflowOutputSchema = z.object({
|
|||
credentialResolutionNote: z.string().optional(),
|
||||
referencedWorkflowIds: z.array(z.string()).optional(),
|
||||
hasUnresolvedPlaceholders: z.boolean().optional(),
|
||||
grouping: groupingOutcomeSchema.optional(),
|
||||
denied: z.boolean().optional(),
|
||||
reason: z.string().optional(),
|
||||
remediation: remediationMetadataSchema.optional(),
|
||||
|
|
@ -530,6 +600,23 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
|
|||
.suspend(confirmationSuspendSchema)
|
||||
.resume(confirmationResumeSchema)
|
||||
.handler(async (input, ctx: BuildCtx) => {
|
||||
const { groupingDecision, groupingReason } = input;
|
||||
if (groupingDecision === 'not_warranted' && !groupingReason?.trim()) {
|
||||
const guidance =
|
||||
"Pass `groupingReason` with `groupingDecision: 'not_warranted'`: say why no valid node group can hold the remaining nodes.";
|
||||
return {
|
||||
success: false,
|
||||
filePath: input.filePath,
|
||||
errors: ['groupingDecision is not_warranted but groupingReason is missing.'],
|
||||
remediation: createRemediation({
|
||||
category: 'code_fixable',
|
||||
shouldEdit: false,
|
||||
reason: 'grouping_reason_missing',
|
||||
guidance,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
let filePath: string;
|
||||
try {
|
||||
// Accepts absolute paths under the workspace root (models often echo
|
||||
|
|
@ -1128,9 +1215,102 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
|
|||
);
|
||||
droppedGroupCount = groupCountBeforeDrop - (json.nodeGroups?.length ?? 0);
|
||||
informational.push(...droppedGroupWarnings);
|
||||
const overCeiling = topLevelItemsWarning(json);
|
||||
|
||||
const topLevel = summarizeWorkflowTopLevelItems(json);
|
||||
const grouping: GroupingOutcome = {
|
||||
topLevelItemCount: topLevel.total,
|
||||
ceiling: topLevel.ceiling,
|
||||
groupCount: topLevel.groupCount,
|
||||
droppedGroupCount,
|
||||
decision: resolveGroupingDecision({
|
||||
groupCount: groupCountBeforeDrop,
|
||||
overCeiling: topLevel.overCeiling,
|
||||
groupingDecision,
|
||||
}),
|
||||
...(groupingReason ? { reason: groupingReason } : {}),
|
||||
};
|
||||
|
||||
// The check applies to canvases this run is responsible for: a new
|
||||
// workflow, a rebuild of one it created, or an edit that pushed a small
|
||||
// workflow over the ceiling. A small edit to a wide user workflow only warns.
|
||||
const snapshotWasUnderCeiling =
|
||||
savedWorkflowSnapshot !== undefined &&
|
||||
!summarizeWorkflowTopLevelItems(savedWorkflowSnapshot).overCeiling;
|
||||
|
||||
// agentExceededCeiling is true when the agent's build made the canvas exceed TOP_LEVEL_ITEM_CEILING boxes;
|
||||
// false when the user's workflow already exceeded it. It gates only the "no groups" refusal.
|
||||
const agentExceededCeiling =
|
||||
!targetWorkflowId ||
|
||||
context.aiCreatedWorkflowIds?.has(targetWorkflowId) === true ||
|
||||
snapshotWasUnderCeiling;
|
||||
|
||||
const refusalReason = groupingDecisionBlocker({
|
||||
summary: topLevel,
|
||||
declaredGroupCount: groupCountBeforeDrop,
|
||||
droppedGroupWarnings,
|
||||
groupingDecision,
|
||||
});
|
||||
|
||||
// The one reason to refuse this build because of grouping, or undefined when the canvas is fine.
|
||||
// A dropped group is the agent's own declaration, so it is refused on any canvas. "No groups"
|
||||
// is refused only when the agent made the canvas exceed the ceiling; on the user's
|
||||
// pre-existing layout it stays a warning.
|
||||
const blocker =
|
||||
agentExceededCeiling || refusalReason?.code === GROUP_DROPPED_OVER_CEILING_CODE
|
||||
? refusalReason
|
||||
: undefined;
|
||||
|
||||
if (blocker) {
|
||||
const groupWasDropped = blocker.code === GROUP_DROPPED_OVER_CEILING_CODE;
|
||||
|
||||
const reason = groupWasDropped
|
||||
? 'workflow_group_dropped_over_ceiling'
|
||||
: 'workflow_grouping_decision_missing';
|
||||
|
||||
const guidance =
|
||||
'Edit the workspace source file so the stages form valid node groups, then call build-workflow again with the same filePath. ' +
|
||||
(groupWasDropped
|
||||
? 'Fix the boundary each dropped-group message names; the opt-out does not apply here.'
|
||||
: "If no valid group can hold the remaining nodes, call it again with groupingDecision: 'not_warranted' and a groupingReason.");
|
||||
|
||||
// The dropped-group error already carries each drop reason, so the matching
|
||||
// warnings would only repeat it.
|
||||
const informationalWithoutDrops = groupWasDropped
|
||||
? informational.filter((warning) => warning.code !== NODE_GROUP_DROPPED_CODE)
|
||||
: informational;
|
||||
|
||||
return await handleValidationFailure({
|
||||
context,
|
||||
blocking: [blocker],
|
||||
informational: informationalWithoutDrops,
|
||||
reason,
|
||||
guidance,
|
||||
summary:
|
||||
'Workflow build stopped: the canvas is over the top-level ceiling and the node groups do not cover it.',
|
||||
binding,
|
||||
sourceHash,
|
||||
targetWorkflowId,
|
||||
filePath,
|
||||
resolvedWorkItemId,
|
||||
resolvedTaskId,
|
||||
plannedTaskId,
|
||||
owner,
|
||||
isSupportingWorkflow,
|
||||
isAuxiliarySupportingWorkflow,
|
||||
withEscalation,
|
||||
stage: 'grouping',
|
||||
grouping,
|
||||
});
|
||||
}
|
||||
|
||||
const overCeiling = topLevelItemsWarning(json, topLevel);
|
||||
if (overCeiling) {
|
||||
informational.push(overCeiling);
|
||||
const accepted = groupingDecision === 'not_warranted' && groupingReason;
|
||||
informational.push(
|
||||
accepted
|
||||
? { ...overCeiling, message: `${overCeiling.message} (accepted: ${groupingReason})` }
|
||||
: overCeiling,
|
||||
);
|
||||
}
|
||||
|
||||
if (await hasLostAllSavedNodeIds(json, targetWorkflowId, context)) {
|
||||
|
|
@ -1301,6 +1481,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
|
|||
hasUnresolvedPlaceholders: hasPlaceholders || undefined,
|
||||
changedNodeNames,
|
||||
executionIntent,
|
||||
grouping,
|
||||
summary,
|
||||
});
|
||||
const postBuildFlow = await directPostBuildFlowHandoff(
|
||||
|
|
@ -1330,6 +1511,14 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
|
|||
isAuxiliarySupportingWorkflow,
|
||||
warningCount: informational.length,
|
||||
droppedGroupCount,
|
||||
...(grouping
|
||||
? {
|
||||
topLevelItemCount: grouping.topLevelItemCount,
|
||||
groupCount: grouping.groupCount,
|
||||
groupingDecision: grouping.decision,
|
||||
groupingReasonProvided: grouping.reason !== undefined,
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
|
||||
return {
|
||||
|
|
@ -1371,6 +1560,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
|
|||
referencedWorkflowIds:
|
||||
referencedWorkflowIds.length > 0 ? referencedWorkflowIds : undefined,
|
||||
hasUnresolvedPlaceholders: hasPlaceholders || undefined,
|
||||
grouping,
|
||||
warnings: combineWarnings(informational.map((w) => formatWarning(w.code, w.message))),
|
||||
};
|
||||
};
|
||||
|
|
|
|||
|
|
@ -79,6 +79,7 @@ export async function reportFailedWorkflowBuildOutcome(
|
|||
errors: string[];
|
||||
summary: string;
|
||||
storeOnRunContext: boolean;
|
||||
grouping?: WorkflowBuildOutcome['grouping'];
|
||||
},
|
||||
): Promise<void> {
|
||||
const buildContext = context.workflowBuildContext;
|
||||
|
|
@ -98,6 +99,7 @@ export async function reportFailedWorkflowBuildOutcome(
|
|||
blockingReason: input.remediation.guidance,
|
||||
failureSignature: input.errors.join('\n').slice(0, 500),
|
||||
remediation: input.remediation,
|
||||
...(input.grouping ? { grouping: input.grouping } : {}),
|
||||
summary: input.summary,
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ export type BuildTelemetryStage =
|
|||
| 'hitl'
|
||||
| 'parse'
|
||||
| 'validation'
|
||||
| 'grouping'
|
||||
| 'name'
|
||||
| 'save'
|
||||
| 'conflict'
|
||||
|
|
@ -29,6 +30,10 @@ export function trackWorkflowSourceBuild(
|
|||
errorCount?: number;
|
||||
warningCount?: number;
|
||||
droppedGroupCount?: number;
|
||||
topLevelItemCount?: number;
|
||||
groupCount?: number;
|
||||
groupingDecision?: 'grouped' | 'not_warranted' | 'under_ceiling' | 'missing';
|
||||
groupingReasonProvided?: boolean;
|
||||
},
|
||||
): void {
|
||||
const buildContext = context.workflowBuildContext;
|
||||
|
|
@ -47,6 +52,14 @@ export function trackWorkflowSourceBuild(
|
|||
error_count: input.errorCount ?? 0,
|
||||
warning_count: input.warningCount ?? 0,
|
||||
dropped_group_count: input.droppedGroupCount ?? 0,
|
||||
...(input.topLevelItemCount !== undefined
|
||||
? {
|
||||
top_level_item_count: input.topLevelItemCount,
|
||||
group_count: input.groupCount ?? 0,
|
||||
grouping_decision: input.groupingDecision ?? 'under_ceiling',
|
||||
grouping_reason_provided: input.groupingReasonProvided === true,
|
||||
}
|
||||
: {}),
|
||||
...(input.targetWorkflowId ? { target_workflow_id: input.targetWorkflowId } : {}),
|
||||
...(input.savedWorkflowId ? { workflow_id: input.savedWorkflowId } : {}),
|
||||
...(input.binding.sourceHash ? { source_hash: input.binding.sourceHash } : {}),
|
||||
|
|
|
|||
|
|
@ -1,17 +1,30 @@
|
|||
import {
|
||||
partitionValidationIssues,
|
||||
toEngineConnections,
|
||||
type IssueSeverity,
|
||||
type WorkflowJSON,
|
||||
} from '@n8n/workflow-sdk';
|
||||
import {
|
||||
isTriggerNodeType,
|
||||
STICKY_NODE_TYPE,
|
||||
TOP_LEVEL_ITEM_CEILING,
|
||||
formatTopLevelItemsMessage,
|
||||
summarizeTopLevelItems,
|
||||
TOP_LEVEL_ITEMS_OVER_CEILING_CODE,
|
||||
type TopLevelItemsSummary,
|
||||
type WorkflowGroupViolation,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
/** Informational: a declared group was invalid and the save removed it. */
|
||||
export const NODE_GROUP_DROPPED_CODE = 'NODE_GROUP_DROPPED';
|
||||
export const TOP_LEVEL_ITEMS_CODE = 'TOP_LEVEL_ITEMS_OVER_CEILING';
|
||||
/** Refusal: the canvas is over the ceiling, has no group, and the agent gave no reason. */
|
||||
export const GROUPING_DECISION_MISSING_CODE = 'GROUPING_DECISION_MISSING';
|
||||
/** Refusal: the canvas is over the ceiling and the save dropped a group the agent declared. */
|
||||
export const GROUP_DROPPED_OVER_CEILING_CODE = 'GROUP_DROPPED_OVER_CEILING';
|
||||
|
||||
/**
|
||||
* What the agent tells build-workflow about groups.
|
||||
* `grouped`: the source declares groups.
|
||||
* `not_warranted`: no group is needed, and a reason is given.
|
||||
*/
|
||||
export type GroupingDecision = 'grouped' | 'not_warranted';
|
||||
|
||||
export interface ValidationWarning {
|
||||
code: string;
|
||||
|
|
@ -50,44 +63,82 @@ export function partitionWarnings(warnings: ValidationWarning[]): {
|
|||
return partitionValidationIssues(warnings);
|
||||
}
|
||||
|
||||
/** Boxes on the canvas with every group collapsed, for the saved shape of a build. */
|
||||
export function summarizeWorkflowTopLevelItems(json: WorkflowJSON): TopLevelItemsSummary {
|
||||
return summarizeTopLevelItems({
|
||||
nodes: json.nodes ?? [],
|
||||
nodeGroups: json.nodeGroups,
|
||||
connectionsBySourceNode: toEngineConnections(json.connections),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Counts the boxes on the canvas with every group collapsed, and warns when there are
|
||||
* more than the TOP_LEVEL_ITEM_CEILING ceiling. Sub-nodes and sticky notes don't count:
|
||||
* a sub-node rides with its parent, and a sticky belongs to the user.
|
||||
* Warns when the collapsed canvas has more boxes than TOP_LEVEL_ITEM_CEILING. The
|
||||
* count lives in n8n-workflow so the MCP tools report the same number.
|
||||
*/
|
||||
export function topLevelItemsWarning(json: WorkflowJSON): ValidationWarning | undefined {
|
||||
const groups = json.nodeGroups ?? [];
|
||||
const groupedNodeIds = new Set(groups.flatMap((group) => group.nodeIds));
|
||||
const subNodeNames = new Set(
|
||||
Object.entries(json.connections ?? {}).flatMap(([nodeName, connectionsByType]) => {
|
||||
const types = Object.keys(connectionsByType);
|
||||
return types.length > 0 && types.every((type) => type !== 'main') ? [nodeName] : [];
|
||||
}),
|
||||
);
|
||||
|
||||
const ungrouped = (json.nodes ?? []).filter(
|
||||
(node) =>
|
||||
!groupedNodeIds.has(node.id) &&
|
||||
node.type !== STICKY_NODE_TYPE &&
|
||||
!(node.name !== undefined && subNodeNames.has(node.name)),
|
||||
);
|
||||
|
||||
const total = groups.length + ungrouped.length;
|
||||
if (total <= TOP_LEVEL_ITEM_CEILING) {
|
||||
export function topLevelItemsWarning(
|
||||
json: WorkflowJSON,
|
||||
summary: TopLevelItemsSummary = summarizeWorkflowTopLevelItems(json),
|
||||
): ValidationWarning | undefined {
|
||||
if (!summary.overCeiling) {
|
||||
return;
|
||||
}
|
||||
|
||||
const groupable = ungrouped
|
||||
.filter((node) => !isTriggerNodeType(node.type))
|
||||
.map((node) => node.name ?? node.id);
|
||||
return {
|
||||
code: TOP_LEVEL_ITEMS_OVER_CEILING_CODE,
|
||||
severity: 'informational',
|
||||
message: formatTopLevelItemsMessage(summary),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The check that blocks a save. Over the ceiling, a dropped group refuses the build
|
||||
* until its boundary is fixed — the agent had already decided to group, and an opt-out
|
||||
* never excuses that. A canvas with no group refuses it until the agent groups or
|
||||
* states why it cannot.
|
||||
*/
|
||||
export function groupingDecisionBlocker(input: {
|
||||
summary: TopLevelItemsSummary;
|
||||
declaredGroupCount: number;
|
||||
droppedGroupWarnings: ValidationWarning[];
|
||||
groupingDecision?: GroupingDecision;
|
||||
}): ValidationWarning | undefined {
|
||||
const { summary, declaredGroupCount, droppedGroupWarnings, groupingDecision } = input;
|
||||
|
||||
if (!summary.overCeiling) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Over the ceiling and the save dropped a group: the agent must repair it, not ship past it.
|
||||
if (droppedGroupWarnings.length > 0) {
|
||||
const reasons = droppedGroupWarnings.map((warning) => warning.message).join(' ');
|
||||
return {
|
||||
code: GROUP_DROPPED_OVER_CEILING_CODE,
|
||||
severity: 'warning',
|
||||
message:
|
||||
`${droppedGroupWarnings.length} of ${declaredGroupCount} declared node group(s) were removed, ` +
|
||||
`so the canvas would have ${summary.total} boxes. ` +
|
||||
`${reasons} Fix the boundary each message names and build again; do not remove the groups.`,
|
||||
};
|
||||
}
|
||||
|
||||
if (summary.groupCount > 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (groupingDecision === 'not_warranted') {
|
||||
return;
|
||||
}
|
||||
|
||||
return {
|
||||
code: TOP_LEVEL_ITEMS_CODE,
|
||||
severity: 'informational',
|
||||
code: GROUPING_DECISION_MISSING_CODE,
|
||||
severity: 'warning',
|
||||
message:
|
||||
`The canvas top level has ${total} boxes with every group collapsed, over the ${TOP_LEVEL_ITEM_CEILING} you should aim for` +
|
||||
(groupable.length > 0 ? `. Still ungrouped: ${groupable.join(', ')}` : '') +
|
||||
'. Group any stage that can form a valid group and build again, or say why each of them cannot join one.',
|
||||
`The canvas would have ${summary.total} boxes with every group collapsed and no node group. ` +
|
||||
`Ungrouped: ${summary.groupableNodeNames.join(', ')}. ` +
|
||||
'Wrap each stage in `.group(name, members, { description })` and build again. ' +
|
||||
"If no valid group can hold these nodes, call build-workflow again with `groupingDecision: 'not_warranted'` " +
|
||||
'and a `groupingReason` that says why.',
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -334,6 +334,21 @@ export const waitGateScriptSchema = z.object({
|
|||
|
||||
export type WaitGateScript = z.infer<typeof waitGateScriptSchema>;
|
||||
|
||||
/**
|
||||
* What the build did about node groups, on the saved (post-drop) shape. A NEW
|
||||
* OPTIONAL FIELD for the same rollback reason as `executionIntent`.
|
||||
*/
|
||||
export const groupingOutcomeSchema = z.object({
|
||||
topLevelItemCount: z.number().int().min(0),
|
||||
ceiling: z.number().int().min(1),
|
||||
groupCount: z.number().int().min(0),
|
||||
droppedGroupCount: z.number().int().min(0),
|
||||
decision: z.enum(['grouped', 'not_warranted', 'under_ceiling', 'missing']),
|
||||
reason: z.string().optional(),
|
||||
});
|
||||
|
||||
export type GroupingOutcome = z.infer<typeof groupingOutcomeSchema>;
|
||||
|
||||
export const workflowBuildOutcomeSchema = z.object({
|
||||
workItemId: z.string(),
|
||||
runId: z.string().optional(),
|
||||
|
|
@ -422,6 +437,8 @@ export const workflowBuildOutcomeSchema = z.object({
|
|||
/** Deterministic setup handoff verdict for post-verification workflow setup. */
|
||||
setupRequirement: workflowSetupRequirementSchema.optional(),
|
||||
remediation: remediationMetadataSchema.optional(),
|
||||
/** Node-group result of this build; absent on outcomes stored before the field existed. */
|
||||
grouping: groupingOutcomeSchema.optional(),
|
||||
/** Count of verify-built-workflow runs for this build; capped by MAX_VERIFY_ATTEMPTS. */
|
||||
verifyAttempts: z.number().int().min(0).optional(),
|
||||
/** Successful verification runs by trigger. A failed rerun removes that trigger's entry. */
|
||||
|
|
|
|||
|
|
@ -147,17 +147,30 @@ value, export that value here.
|
|||
|
||||
```ts
|
||||
// src/my-feature.module.ts
|
||||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
|
||||
export const MyFeatureModule: FrontendModuleDescription = {
|
||||
export const MyFeatureModule = defineFrontendModule({
|
||||
// Must match the backend module id: both gate off `/rest/module-settings`.
|
||||
id: 'my-feature',
|
||||
name: 'My Feature',
|
||||
description: 'What this module does',
|
||||
icon: 'box',
|
||||
};
|
||||
});
|
||||
```
|
||||
|
||||
Declare every descriptor with `defineFrontendModule()`. It is the canonical form.
|
||||
It returns the object that you give it, and it changes no behaviour. It gives you
|
||||
two things:
|
||||
|
||||
- One seam. The SDK gets one function to attach validation or a dev-mode check
|
||||
to. Ten annotated object literals give it none.
|
||||
- Inference. Each field keeps its literal type, so `MyFeatureModule.id` reads as
|
||||
`'my-feature'` and not as `string`.
|
||||
|
||||
Do not annotate the descriptor also. `defineFrontendModule()` checks the object
|
||||
against `FrontendModuleDescription`, and the annotation makes the types wide
|
||||
again.
|
||||
|
||||
The `id` field is critical. `settingsStore.isModuleActive(id)` reads the `activeModules` list from
|
||||
the backend. An id with no backend twin is never active. A route that uses the module availability
|
||||
guard then does not resolve.
|
||||
|
|
|
|||
|
|
@ -133,6 +133,15 @@ describe('createFrontend', () => {
|
|||
expect(readFileSync(join(packageDir, `src/${NAME}.store.ts`), 'utf8')).toContain(
|
||||
"defineStore('myFeature'",
|
||||
);
|
||||
|
||||
// `defineFrontendModule()` is the canonical descriptor form: it is the SDK's
|
||||
// one seam for validation and dev-mode checks. A scaffold that emits a bare
|
||||
// annotated object starts every new module outside that seam.
|
||||
const descriptor = readFileSync(join(packageDir, `src/${NAME}.module.ts`), 'utf8');
|
||||
expect(descriptor).toContain(
|
||||
"import { defineFrontendModule } from '@n8n/frontend-module-sdk';",
|
||||
);
|
||||
expect(descriptor).toContain('export const MyFeatureModule = defineFrontendModule({');
|
||||
});
|
||||
|
||||
it('registers the plugins a design-system consumer needs, and declares them', () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
|
||||
/**
|
||||
* Keep this file import-light: types and the SDK only. Views load lazily
|
||||
|
|
@ -6,7 +6,7 @@ import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
|||
* referenced inside route guards or handlers, never at module scope — a
|
||||
* top-level store import pulls the whole module in even when it is disabled.
|
||||
*/
|
||||
export const {{PascalName}}Module: FrontendModuleDescription = {
|
||||
export const {{PascalName}}Module = defineFrontendModule({
|
||||
// Must match the backend module id: both gate off `/rest/module-settings`.
|
||||
id: '{{name}}',
|
||||
name: '{{TitleName}}',
|
||||
|
|
@ -26,4 +26,4 @@ export const {{PascalName}}Module: FrontendModuleDescription = {
|
|||
|
||||
// The SDK types the fields below, but no host in the shell reads them yet.
|
||||
// A value here does nothing at runtime: locales, shortcuts, banners, setup.
|
||||
};
|
||||
});
|
||||
|
|
|
|||
106
packages/@n8n/telemetry/src/events/node-type-policies.ts
Normal file
106
packages/@n8n/telemetry/src/events/node-type-policies.ts
Normal file
|
|
@ -0,0 +1,106 @@
|
|||
import { z } from 'zod/v4';
|
||||
|
||||
import { defineTelemetryEvents } from '../define';
|
||||
|
||||
/**
|
||||
* Absent for a write that no user made — an environment bootstrap writes the policy with no
|
||||
* actor, and `user_id` is what builds the RudderStack user, so a placeholder would create one.
|
||||
*/
|
||||
const userId = z.string().optional();
|
||||
|
||||
const source = z
|
||||
.enum(['user', 'environment'])
|
||||
.describe("'environment' is a bootstrap write from configuration, with no acting user");
|
||||
|
||||
const scope = z
|
||||
.enum(['instance', 'project'])
|
||||
.describe('Which scope the policy was written at; project policies compose under instance');
|
||||
|
||||
const projectId = z.string().optional().describe('Absent at instance scope');
|
||||
|
||||
const ruleCounts = {
|
||||
rule_count: z.number(),
|
||||
allow_rule_count: z.number(),
|
||||
deny_rule_count: z.number(),
|
||||
delegate_rule_count: z
|
||||
.number()
|
||||
.describe('Always 0 at project scope, which rejects the delegate action'),
|
||||
};
|
||||
|
||||
export const NODE_TYPE_POLICIES_TELEMETRY = defineTelemetryEvents({
|
||||
USER_SAVED_NODE_TYPE_POLICY: {
|
||||
name: 'User saved node type policy',
|
||||
description:
|
||||
'A node type availability policy was saved for one scope through the composed write that the policy UI uses. One event per save, covering both the scope default action and its rules. Rule-level edits through the advanced document and attachment APIs report separately. The type counts and list report what this scope decides on its own, so at project scope they are the project layer before it composes with the instance policy.',
|
||||
properties: z.object({
|
||||
user_id: userId,
|
||||
source,
|
||||
scope,
|
||||
project_id: projectId,
|
||||
default_action: z
|
||||
.enum(['allow', 'deny', 'delegate'])
|
||||
.describe('What the scope decides for a type that no rule matches'),
|
||||
previous_default_action: z
|
||||
.enum(['allow', 'deny', 'delegate'])
|
||||
.nullable()
|
||||
.describe('Null on the first write to this scope, which had no stored default'),
|
||||
is_first_write: z
|
||||
.boolean()
|
||||
.describe('Whether this write created the scope; an unwritten scope allows every type'),
|
||||
...ruleCounts,
|
||||
name_selector_count: z.number().describe('Rules that target one exact node type name'),
|
||||
package_selector_count: z.number().describe('Rules that target a whole node package'),
|
||||
evaluated_type_count: z
|
||||
.number()
|
||||
.describe('Node types this instance knows, and so the denominator for the counts below'),
|
||||
blocked_type_count: z.number(),
|
||||
allowed_type_count: z.number(),
|
||||
delegated_type_count: z
|
||||
.number()
|
||||
.describe('Types a project can opt into; always 0 at project scope'),
|
||||
blocked_types: z
|
||||
.array(z.string())
|
||||
.describe(
|
||||
'The node types this policy makes unavailable, capped at 100. Node type names, not user data. Compare the length against blocked_type_count to see whether the cap dropped any',
|
||||
),
|
||||
allowed_types: z
|
||||
.array(z.string())
|
||||
.describe('The node types this policy leaves available, capped at 100 the same way'),
|
||||
previous_rule_count: z.number().nullable().describe('Null on the first write to this scope'),
|
||||
shadow_warning_count: z
|
||||
.number()
|
||||
.describe('Saved rules that can never match, because an earlier rule already covers them'),
|
||||
version: z.number().describe('The scope version after the write'),
|
||||
}),
|
||||
},
|
||||
USER_UPDATED_NODE_TYPE_POLICY_DOCUMENT: {
|
||||
name: 'User updated node type policy document',
|
||||
description:
|
||||
'A policy document was created, edited, or deleted through the advanced instance-only document API. The composed save that the policy UI uses reports as "User saved node type policy" instead. Rule counts describe the document after the operation, so a delete reports 0 and carries the deleted size in previous_rule_count.',
|
||||
properties: z.object({
|
||||
user_id: userId,
|
||||
source,
|
||||
operation: z.enum(['created', 'updated', 'deleted']),
|
||||
policy_id: z.string(),
|
||||
...ruleCounts,
|
||||
previous_rule_count: z.number().nullable().describe('Null on a created document'),
|
||||
}),
|
||||
},
|
||||
USER_UPDATED_NODE_TYPE_POLICY_ATTACHMENTS: {
|
||||
name: 'User updated node type policy attachments',
|
||||
description:
|
||||
'The set of policy documents attached to one scope was replaced through the advanced instance-only attachment API. Reports the resulting set, not the individual add or remove.',
|
||||
properties: z.object({
|
||||
user_id: userId,
|
||||
source,
|
||||
scope,
|
||||
project_id: projectId,
|
||||
scope_id: z.string(),
|
||||
attachment_count: z.number(),
|
||||
floor_attachment_count: z
|
||||
.number()
|
||||
.describe('Attachments that a narrower scope cannot override'),
|
||||
previous_attachment_count: z.number(),
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
|
@ -4,6 +4,7 @@ import { CREDENTIALS_TELEMETRY } from './events/credentials';
|
|||
import { INSTANCE_TELEMETRY } from './events/instance';
|
||||
import { INSTANCE_AI_TELEMETRY } from './events/instance-ai';
|
||||
import { MCP_TELEMETRY } from './events/mcp';
|
||||
import { NODE_TYPE_POLICIES_TELEMETRY } from './events/node-type-policies';
|
||||
import { PLATFORM_TELEMETRY } from './events/platform';
|
||||
import { WORKFLOW_TELEMETRY } from './events/workflow';
|
||||
import { WORKFLOW_REVIEWS_TELEMETRY } from './events/workflow-reviews';
|
||||
|
|
@ -15,6 +16,7 @@ export const TELEMETRY_EVENT = {
|
|||
INSTANCE: INSTANCE_TELEMETRY,
|
||||
INSTANCE_AI: INSTANCE_AI_TELEMETRY,
|
||||
MCP: MCP_TELEMETRY,
|
||||
NODE_TYPE_POLICIES: NODE_TYPE_POLICIES_TELEMETRY,
|
||||
WORKFLOW: WORKFLOW_TELEMETRY,
|
||||
WORKFLOW_REVIEWS: WORKFLOW_REVIEWS_TELEMETRY,
|
||||
} satisfies TelemetryEventRegistry;
|
||||
|
|
|
|||
|
|
@ -120,6 +120,13 @@ describe('NODE_GROUPS_REFERENCE', () => {
|
|||
expect(NODE_GROUPS_REFERENCE).toMatch(/keep the .+ and their descriptions\s+intact/is);
|
||||
});
|
||||
|
||||
it('names the two shapes that are cut wrong most often', () => {
|
||||
// Case 8 (gates in a row) and case 3 (two-node stages) failed on every eval run
|
||||
// while the abstract rule was present, so the guidance names the shape.
|
||||
expect(GROUPING_GUIDANCE).toMatch(/first node of the next group, never loose/i);
|
||||
expect(GROUPING_GUIDANCE).toMatch(/merge the small stage into its neighbour/i);
|
||||
});
|
||||
|
||||
it('tells agents to fix what a dropped-group warning reports, not the boundary', () => {
|
||||
// A build took two dropped-group warnings as licence to publish the stage
|
||||
// ungrouped instead of fixing the group.
|
||||
|
|
|
|||
|
|
@ -127,13 +127,13 @@ Every workflow needs an explicit grouping decision, taken while you write the co
|
|||
|
||||
- **When:** count top-level items with no groups (trigger + every node or existing group). More than ${TOP_LEVEL_ITEM_CEILING} → you must group. A linear pipeline is the normal case for grouping, not an exemption: stages run in sequence (ingest → transform → deliver). ${TOP_LEVEL_ITEM_CEILING} or fewer serving one objective → leave it ungrouped.
|
||||
- **How many:** one per stage or high-level objective, typically 3-5, aiming for at most ${TOP_LEVEL_ITEM_CEILING} top-level items after grouping. Staying above ${TOP_LEVEL_ITEM_CEILING} is only acceptable when every item still at the top level is either a group already, or a node that cannot join one without breaking a validity rule — check each one before you settle. Never split one objective to hit the number. When in doubt, fewer and larger.
|
||||
- **What belongs together:** one business outcome ("Fetch new recordings"), never a technical category ("HTTP requests", "Database operations"). Cut where the objective changes; merge groups serving the same outcome. Stages of one or two nodes mean the boundaries are too fine — widen them.
|
||||
- **What belongs together:** one business outcome ("Fetch new recordings"), never a technical category ("HTTP requests", "Database operations"). Cut where the objective changes; merge groups serving the same outcome. Stages of one or two nodes mean the boundaries are too fine — widen them: merge the small stage into its neighbour rather than keeping three groups.
|
||||
- **Groups vs sub-workflows:** a group organises one canvas; a sub-workflow is separately executed and reusable. Group to make one canvas readable; extract a sub-workflow to reuse logic or isolate execution.
|
||||
|
||||
**Boundaries:** a group takes one member receiving from outside and one member sending outside; any number of connections may reach those two members.
|
||||
|
||||
- A branch that stops one way and continues the other: keep the branch node inside with both its paths outside, end the group before it, or leave it and its stop path outside.
|
||||
- Work that fans out into parallel branches: the node they fan out from and the node they reconverge on both belong inside, or every branch faces outward on its own.
|
||||
- Work that fans out into parallel branches: the node they fan out from and the node they reconverge on both belong inside, or every branch faces outward on its own. Several IF nodes in a row: each IF sits inside a stage; the node where their paths join is inside that stage or the first node of the next group, never loose.
|
||||
- When the node at either end cannot be a member — the trigger, or a node already in another group — the stage needs its own step there to serve as the single entry or exit. Use a step the stage already has; add a plain pass-through only when the stage would otherwise stay ungrouped.
|
||||
- A rejected group never closes the stage: try a smaller slice that is valid — one branch, or the linear run before or after the split — before leaving anything ungrouped.
|
||||
|
||||
|
|
|
|||
|
|
@ -64,6 +64,7 @@ describe('DeprecationService', () => {
|
|||
['N8N_CONFIG_FILES', '1', true],
|
||||
['N8N_SKIP_WEBHOOK_DEREGISTRATION_SHUTDOWN', '1', true],
|
||||
['N8N_RUNNERS_ENABLED', '1', true],
|
||||
['N8N_DB_PING_TIMEOUT', '1', true],
|
||||
['WEBHOOK_URL', 'https://example.com/', true],
|
||||
['N8N_DEFAULT_BINARY_DATA_MODE', 'default', true],
|
||||
['N8N_DEFAULT_BINARY_DATA_MODE', 'filesystem', false],
|
||||
|
|
|
|||
|
|
@ -41,6 +41,10 @@ export class DeprecationService {
|
|||
{ envVar: 'N8N_AVAILABLE_BINARY_DATA_MODES', message: SAFE_TO_REMOVE },
|
||||
{ envVar: 'N8N_CONFIG_FILES', message: 'Please use .env files or *_FILE env vars instead.' },
|
||||
{ envVar: 'N8N_RUNNERS_ENABLED', message: SAFE_TO_REMOVE },
|
||||
{
|
||||
envVar: 'N8N_DB_PING_TIMEOUT',
|
||||
message: 'Use DB_PING_TIMEOUT_MS instead. This variable will be removed in a future version.',
|
||||
},
|
||||
{
|
||||
envVar: 'N8N_SKIP_WEBHOOK_DEREGISTRATION_SHUTDOWN',
|
||||
message: `n8n no longer deregisters webhooks at startup and shutdown. ${SAFE_TO_REMOVE}`,
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ import type { RelayEventMap } from '@/events/maps/relay.event-map';
|
|||
import { TelemetryEventRelay, getSemanticVersioning } from '@/events/relays/telemetry.event-relay';
|
||||
import type { License } from '@/license';
|
||||
import { OtelConfig } from '@/modules/otel/otel.config';
|
||||
import type { PolicyRule } from '@/modules/type-availability-policies/policy-rule.types';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
import type { Telemetry } from '@/telemetry';
|
||||
|
||||
|
|
@ -661,6 +662,360 @@ describe('TelemetryEventRelay', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('node type policy events', () => {
|
||||
const denyRule: PolicyRule = {
|
||||
id: 'rule-1',
|
||||
action: 'deny',
|
||||
selector: { kind: 'name', value: 'n8n-nodes-base.executeCommand' },
|
||||
};
|
||||
const allowPackageRule: PolicyRule = {
|
||||
id: 'rule-2',
|
||||
action: 'allow',
|
||||
selector: { kind: 'package', value: 'n8n-nodes-base' },
|
||||
};
|
||||
const delegateRule: PolicyRule = {
|
||||
id: 'rule-3',
|
||||
action: 'delegate',
|
||||
selector: { kind: 'name', value: 'n8n-nodes-base.code' },
|
||||
};
|
||||
|
||||
const knownTypes = (...names: string[]) =>
|
||||
Object.fromEntries(names.map((name) => [name, {}])) as ReturnType<NodeTypes['getKnownTypes']>;
|
||||
|
||||
beforeEach(() => {
|
||||
nodeTypes.getKnownTypes.mockReturnValue(
|
||||
knownTypes(
|
||||
'n8n-nodes-base.code',
|
||||
'n8n-nodes-base.executeCommand',
|
||||
'@acme/n8n-nodes-acme.thing',
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('should track a first instance-scope save', () => {
|
||||
const event: RelayEventMap['node-type-policy-saved'] = {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
projectId: null,
|
||||
scopeId: 'scope-1',
|
||||
before: null,
|
||||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [denyRule, allowPackageRule, delegateRule],
|
||||
warningCount: 2,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_SAVED_NODE_TYPE_POLICY,
|
||||
{
|
||||
user_id: 'user123',
|
||||
source: 'user',
|
||||
scope: 'instance',
|
||||
default_action: 'deny',
|
||||
previous_default_action: null,
|
||||
is_first_write: true,
|
||||
rule_count: 3,
|
||||
allow_rule_count: 1,
|
||||
deny_rule_count: 1,
|
||||
delegate_rule_count: 1,
|
||||
name_selector_count: 2,
|
||||
package_selector_count: 1,
|
||||
evaluated_type_count: 3,
|
||||
blocked_type_count: 2,
|
||||
allowed_type_count: 1,
|
||||
delegated_type_count: 0,
|
||||
blocked_types: ['n8n-nodes-base.executeCommand', '@acme/n8n-nodes-acme.thing'],
|
||||
allowed_types: ['n8n-nodes-base.code'],
|
||||
previous_rule_count: null,
|
||||
shadow_warning_count: 2,
|
||||
version: 1,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should track a project-scope save over an existing policy', () => {
|
||||
const event: RelayEventMap['node-type-policy-saved'] = {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
projectId: 'project-1',
|
||||
scopeId: 'scope-2',
|
||||
before: { defaultAction: 'allow', version: 4 },
|
||||
after: { defaultAction: 'deny', version: 5 },
|
||||
rulesBefore: [denyRule],
|
||||
rulesAfter: [allowPackageRule],
|
||||
warningCount: 0,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_SAVED_NODE_TYPE_POLICY,
|
||||
expect.objectContaining({
|
||||
scope: 'project',
|
||||
project_id: 'project-1',
|
||||
previous_default_action: 'allow',
|
||||
is_first_write: false,
|
||||
previous_rule_count: 1,
|
||||
rule_count: 1,
|
||||
allow_rule_count: 1,
|
||||
version: 5,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should report an environment write as a source instead of a user', () => {
|
||||
const event: RelayEventMap['node-type-policy-saved'] = {
|
||||
updatedBy: 'environment',
|
||||
kind: 'node-types',
|
||||
projectId: null,
|
||||
scopeId: 'scope-1',
|
||||
before: null,
|
||||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [],
|
||||
warningCount: 0,
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-saved', event);
|
||||
|
||||
const [, properties] = vi.mocked(telemetry.track).mock.calls.at(-1)!;
|
||||
expect(properties).not.toHaveProperty('user_id');
|
||||
expect(properties).toMatchObject({ source: 'environment' });
|
||||
});
|
||||
|
||||
it('should track a created policy document', () => {
|
||||
const event: RelayEventMap['node-type-policy-document-created'] = {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
policyId: 'policy-1',
|
||||
origin: 'document-api',
|
||||
after: { rules: [denyRule], version: 1 },
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-document-created', event);
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_UPDATED_NODE_TYPE_POLICY_DOCUMENT,
|
||||
{
|
||||
user_id: 'user123',
|
||||
source: 'user',
|
||||
operation: 'created',
|
||||
policy_id: 'policy-1',
|
||||
rule_count: 1,
|
||||
allow_rule_count: 0,
|
||||
deny_rule_count: 1,
|
||||
delegate_rule_count: 0,
|
||||
previous_rule_count: null,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('should track an updated policy document', () => {
|
||||
const event: RelayEventMap['node-type-policy-document-updated'] = {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
policyId: 'policy-1',
|
||||
origin: 'document-api',
|
||||
before: { rules: [denyRule], version: 1 },
|
||||
after: { rules: [denyRule, allowPackageRule], version: 2 },
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-document-updated', event);
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_UPDATED_NODE_TYPE_POLICY_DOCUMENT,
|
||||
expect.objectContaining({
|
||||
operation: 'updated',
|
||||
rule_count: 2,
|
||||
previous_rule_count: 1,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should report a deleted policy document as empty, keeping its prior size', () => {
|
||||
const event: RelayEventMap['node-type-policy-document-deleted'] = {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
policyId: 'policy-1',
|
||||
before: { rules: [denyRule, allowPackageRule], version: 3 },
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-document-deleted', event);
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_UPDATED_NODE_TYPE_POLICY_DOCUMENT,
|
||||
expect.objectContaining({
|
||||
operation: 'deleted',
|
||||
rule_count: 0,
|
||||
deny_rule_count: 0,
|
||||
previous_rule_count: 2,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should list the blocked types when the policy allows by default', () => {
|
||||
eventService.emit('node-type-policy-saved', {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
projectId: null,
|
||||
scopeId: 'scope-1',
|
||||
before: null,
|
||||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [denyRule],
|
||||
warningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_SAVED_NODE_TYPE_POLICY,
|
||||
expect.objectContaining({
|
||||
blocked_type_count: 1,
|
||||
allowed_type_count: 2,
|
||||
blocked_types: ['n8n-nodes-base.executeCommand'],
|
||||
allowed_types: ['n8n-nodes-base.code', '@acme/n8n-nodes-acme.thing'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should expand a package rule into the types it actually blocks', () => {
|
||||
eventService.emit('node-type-policy-saved', {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
projectId: null,
|
||||
scopeId: 'scope-1',
|
||||
before: null,
|
||||
after: { defaultAction: 'allow', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [{ ...allowPackageRule, action: 'deny' }],
|
||||
warningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_SAVED_NODE_TYPE_POLICY,
|
||||
expect.objectContaining({
|
||||
rule_count: 1,
|
||||
blocked_type_count: 2,
|
||||
blocked_types: ['n8n-nodes-base.code', 'n8n-nodes-base.executeCommand'],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should cap the listed types and flag that it did', () => {
|
||||
const names = Array.from({ length: 250 }, (_, i) => `n8n-nodes-base.node${i}`);
|
||||
nodeTypes.getKnownTypes.mockReturnValue(knownTypes(...names));
|
||||
|
||||
eventService.emit('node-type-policy-saved', {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
projectId: null,
|
||||
scopeId: 'scope-1',
|
||||
before: null,
|
||||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: names.slice(0, 120).map((name, i) => ({
|
||||
id: `allow-${i}`,
|
||||
action: 'allow' as const,
|
||||
selector: { kind: 'name' as const, value: name },
|
||||
})),
|
||||
warningCount: 0,
|
||||
});
|
||||
|
||||
const properties = vi.mocked(telemetry.track).mock.calls.at(-1)?.[1];
|
||||
expect(properties).toMatchObject({
|
||||
evaluated_type_count: 250,
|
||||
allowed_type_count: 120,
|
||||
blocked_type_count: 130,
|
||||
});
|
||||
expect(properties?.blocked_types).toHaveLength(100);
|
||||
expect(properties?.allowed_types).toHaveLength(100);
|
||||
});
|
||||
|
||||
it('should report every known type as blocked when nothing is allowed', () => {
|
||||
eventService.emit('node-type-policy-saved', {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
projectId: null,
|
||||
scopeId: 'scope-1',
|
||||
before: null,
|
||||
after: { defaultAction: 'deny', version: 1 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [],
|
||||
warningCount: 0,
|
||||
});
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_SAVED_NODE_TYPE_POLICY,
|
||||
expect.objectContaining({
|
||||
blocked_type_count: 3,
|
||||
allowed_type_count: 0,
|
||||
blocked_types: [
|
||||
'n8n-nodes-base.code',
|
||||
'n8n-nodes-base.executeCommand',
|
||||
'@acme/n8n-nodes-acme.thing',
|
||||
],
|
||||
allowed_types: [],
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it.each(['created', 'updated'] as const)(
|
||||
'should not report a composed save as a %s document, which would double-count it',
|
||||
(operation) => {
|
||||
eventService.emit(`node-type-policy-document-${operation}`, {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
policyId: 'policy-1',
|
||||
origin: 'composed-save',
|
||||
before: { rules: [], version: 1 },
|
||||
after: { rules: [denyRule], version: 2 },
|
||||
});
|
||||
|
||||
expect(telemetry.track).not.toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_UPDATED_NODE_TYPE_POLICY_DOCUMENT,
|
||||
expect.anything(),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('should track replaced attachments', () => {
|
||||
const event: RelayEventMap['node-type-policy-attachments-updated'] = {
|
||||
updatedBy: 'user123',
|
||||
kind: 'node-types',
|
||||
projectId: 'project-1',
|
||||
scopeId: 'scope-2',
|
||||
before: {
|
||||
attachments: [{ policyId: 'policy-1', rules: [], priority: 0, isFloor: false }],
|
||||
version: 1,
|
||||
},
|
||||
after: {
|
||||
attachments: [
|
||||
{ policyId: 'policy-1', rules: [], priority: 0, isFloor: false },
|
||||
{ policyId: 'policy-2', rules: [denyRule], priority: 0, isFloor: true },
|
||||
],
|
||||
version: 2,
|
||||
},
|
||||
};
|
||||
|
||||
eventService.emit('node-type-policy-attachments-updated', event);
|
||||
|
||||
expect(telemetry.track).toHaveBeenCalledWith(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_UPDATED_NODE_TYPE_POLICY_ATTACHMENTS,
|
||||
{
|
||||
user_id: 'user123',
|
||||
source: 'user',
|
||||
scope: 'project',
|
||||
project_id: 'project-1',
|
||||
scope_id: 'scope-2',
|
||||
attachment_count: 2,
|
||||
floor_attachment_count: 1,
|
||||
previous_attachment_count: 1,
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('external secrets events', () => {
|
||||
it('should track on `external-secrets-provider-settings-saved` event', () => {
|
||||
const event: RelayEventMap['external-secrets-provider-settings-saved'] = {
|
||||
|
|
|
|||
|
|
@ -51,6 +51,13 @@ export type UserLike = {
|
|||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* Which write path produced a policy document event. A composed save emits a document event
|
||||
* of its own, so a consumer that already reports the composed save uses this to skip it
|
||||
* instead of counting one save twice.
|
||||
*/
|
||||
export type PolicyWriteOrigin = 'composed-save' | 'document-api';
|
||||
|
||||
export type ProjectSummary = {
|
||||
id: string;
|
||||
name: string;
|
||||
|
|
@ -1317,6 +1324,7 @@ export type RelayEventMap = {
|
|||
updatedBy: string;
|
||||
kind: string;
|
||||
policyId: string;
|
||||
origin: PolicyWriteOrigin;
|
||||
after: { rules: readonly PolicyRule[]; version: number };
|
||||
};
|
||||
|
||||
|
|
@ -1324,6 +1332,7 @@ export type RelayEventMap = {
|
|||
updatedBy: string;
|
||||
kind: string;
|
||||
policyId: string;
|
||||
origin: PolicyWriteOrigin;
|
||||
before: { rules: readonly PolicyRule[]; version: number };
|
||||
after: { rules: readonly PolicyRule[]; version: number };
|
||||
};
|
||||
|
|
@ -1335,6 +1344,23 @@ export type RelayEventMap = {
|
|||
before: { rules: readonly PolicyRule[]; version: number };
|
||||
};
|
||||
|
||||
/**
|
||||
* One composed save of a scope's whole effective policy: its default action and its rules.
|
||||
* Emitted alongside the granular scope and document events, which the audit log needs, so a
|
||||
* consumer that wants one row per save listens to this one instead of joining those two.
|
||||
*/
|
||||
'node-type-policy-saved': {
|
||||
updatedBy: string;
|
||||
kind: string;
|
||||
projectId: string | null;
|
||||
scopeId: string;
|
||||
before: { defaultAction: PolicyAction; version: number } | null;
|
||||
after: { defaultAction: PolicyAction; version: number };
|
||||
rulesBefore: readonly PolicyRule[] | null;
|
||||
rulesAfter: readonly PolicyRule[];
|
||||
warningCount: number;
|
||||
};
|
||||
|
||||
'node-type-policy-attachments-updated': {
|
||||
updatedBy: string;
|
||||
kind: string;
|
||||
|
|
|
|||
|
|
@ -39,6 +39,11 @@ import type { RelayEventMap } from '@/events/maps/relay.event-map';
|
|||
import { determineFinalExecutionStatus } from '@/execution-lifecycle/shared/shared-hook-functions';
|
||||
import type { IExecutionTrackProperties } from '@/interfaces';
|
||||
import { License } from '@/license';
|
||||
import { partitionTypesByAction } from '@/modules/type-availability-policies/policy-evaluator';
|
||||
import type {
|
||||
PolicyAction,
|
||||
PolicyRule,
|
||||
} from '@/modules/type-availability-policies/policy-rule.types';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
|
||||
import { EventRelay } from './event-relay';
|
||||
|
|
@ -59,6 +64,66 @@ function countNodeCustomTelemetryTags(nodes: INode[]): number {
|
|||
return nodes.reduce((total, node) => total + (node.customTelemetryTags?.tag?.length ?? 0), 0);
|
||||
}
|
||||
|
||||
/**
|
||||
* A policy write has no acting user when configuration bootstraps it. `user_id` builds the
|
||||
* RudderStack user, so the literal actor is reported as a source instead of as a user id.
|
||||
*/
|
||||
function policyActor(updatedBy: string): { user_id?: string; source: 'user' | 'environment' } {
|
||||
return updatedBy === 'environment'
|
||||
? { source: 'environment' }
|
||||
: { user_id: updatedBy, source: 'user' };
|
||||
}
|
||||
|
||||
function policyScope(projectId: string | null): {
|
||||
scope: 'instance' | 'project';
|
||||
project_id?: string;
|
||||
} {
|
||||
return projectId === null ? { scope: 'instance' } : { scope: 'project', project_id: projectId };
|
||||
}
|
||||
|
||||
function countRuleActions(rules: readonly PolicyRule[]) {
|
||||
return {
|
||||
rule_count: rules.length,
|
||||
allow_rule_count: rules.filter((rule) => rule.action === 'allow').length,
|
||||
deny_rule_count: rules.filter((rule) => rule.action === 'deny').length,
|
||||
delegate_rule_count: rules.filter((rule) => rule.action === 'delegate').length,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* A default-deny policy blocks nearly every known type, so an uncapped list would blow the
|
||||
* 32 KB payload cap. The counts alongside each list carry the total either way.
|
||||
*/
|
||||
const MAX_LISTED_POLICY_TYPES = 100;
|
||||
|
||||
/**
|
||||
* What a saved policy makes of every node type this instance knows. Runs the same evaluation
|
||||
* the node panel runs, once per save rather than once per workflow open.
|
||||
*/
|
||||
function summarizeTypeAvailability(
|
||||
rules: readonly PolicyRule[],
|
||||
defaultAction: PolicyAction,
|
||||
typeNames: readonly string[],
|
||||
) {
|
||||
const partition = partitionTypesByAction(rules, defaultAction, typeNames);
|
||||
|
||||
return {
|
||||
evaluated_type_count: typeNames.length,
|
||||
blocked_type_count: partition.deny.length,
|
||||
allowed_type_count: partition.allow.length,
|
||||
delegated_type_count: partition.delegate.length,
|
||||
blocked_types: partition.deny.slice(0, MAX_LISTED_POLICY_TYPES),
|
||||
allowed_types: partition.allow.slice(0, MAX_LISTED_POLICY_TYPES),
|
||||
};
|
||||
}
|
||||
|
||||
function countSelectorKinds(rules: readonly PolicyRule[]) {
|
||||
return {
|
||||
name_selector_count: rules.filter((rule) => rule.selector.kind === 'name').length,
|
||||
package_selector_count: rules.filter((rule) => rule.selector.kind === 'package').length,
|
||||
};
|
||||
}
|
||||
|
||||
function limitNodeGraphStringSize(nodeGraphString: string): string {
|
||||
if (Buffer.byteLength(nodeGraphString, 'utf8') > MAX_NODE_GRAPH_STRING_SIZE) return '{}';
|
||||
|
||||
|
|
@ -124,6 +189,12 @@ export class TelemetryEventRelay extends EventRelay {
|
|||
'variable-created': (event) => this.variableCreated(event),
|
||||
'variable-updated': (event) => this.variableUpdated(event),
|
||||
'variable-deleted': (event) => this.variableDeleted(event),
|
||||
'node-type-policy-saved': (event) => this.nodeTypePolicySaved(event),
|
||||
'node-type-policy-document-created': (event) => this.nodeTypePolicyDocumentCreated(event),
|
||||
'node-type-policy-document-updated': (event) => this.nodeTypePolicyDocumentUpdated(event),
|
||||
'node-type-policy-document-deleted': (event) => this.nodeTypePolicyDocumentDeleted(event),
|
||||
'node-type-policy-attachments-updated': (event) =>
|
||||
this.nodeTypePolicyAttachmentsUpdated(event),
|
||||
'external-secrets-provider-settings-saved': (event) =>
|
||||
this.externalSecretsProviderSettingsSaved(event),
|
||||
'external-secrets-provider-reloaded': (event) => this.externalSecretsProviderReloaded(event),
|
||||
|
|
@ -453,6 +524,112 @@ export class TelemetryEventRelay extends EventRelay {
|
|||
|
||||
// #endregion
|
||||
|
||||
// #region Node type policy
|
||||
|
||||
private nodeTypePolicySaved({
|
||||
updatedBy,
|
||||
projectId,
|
||||
before,
|
||||
after,
|
||||
rulesBefore,
|
||||
rulesAfter,
|
||||
warningCount,
|
||||
}: RelayEventMap['node-type-policy-saved']) {
|
||||
this.telemetry.track(TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_SAVED_NODE_TYPE_POLICY, {
|
||||
...policyActor(updatedBy),
|
||||
...policyScope(projectId),
|
||||
default_action: after.defaultAction,
|
||||
previous_default_action: before?.defaultAction ?? null,
|
||||
is_first_write: before === null,
|
||||
...countRuleActions(rulesAfter),
|
||||
...countSelectorKinds(rulesAfter),
|
||||
...summarizeTypeAvailability(
|
||||
rulesAfter,
|
||||
after.defaultAction,
|
||||
Object.keys(this.nodeTypes.getKnownTypes()),
|
||||
),
|
||||
previous_rule_count: rulesBefore?.length ?? null,
|
||||
shadow_warning_count: warningCount,
|
||||
version: after.version,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* A composed save reports itself, and also emits a document event for the audit log. Only
|
||||
* the advanced document API reaches telemetry here, so one save stays one row.
|
||||
*/
|
||||
private nodeTypePolicyDocumentCreated({
|
||||
updatedBy,
|
||||
policyId,
|
||||
origin,
|
||||
after,
|
||||
}: RelayEventMap['node-type-policy-document-created']) {
|
||||
if (origin === 'composed-save') return;
|
||||
|
||||
this.trackPolicyDocument(updatedBy, policyId, 'created', after.rules, null);
|
||||
}
|
||||
|
||||
private nodeTypePolicyDocumentUpdated({
|
||||
updatedBy,
|
||||
policyId,
|
||||
origin,
|
||||
before,
|
||||
after,
|
||||
}: RelayEventMap['node-type-policy-document-updated']) {
|
||||
if (origin === 'composed-save') return;
|
||||
|
||||
this.trackPolicyDocument(updatedBy, policyId, 'updated', after.rules, before.rules);
|
||||
}
|
||||
|
||||
private nodeTypePolicyDocumentDeleted({
|
||||
updatedBy,
|
||||
policyId,
|
||||
before,
|
||||
}: RelayEventMap['node-type-policy-document-deleted']) {
|
||||
this.trackPolicyDocument(updatedBy, policyId, 'deleted', [], before.rules);
|
||||
}
|
||||
|
||||
private trackPolicyDocument(
|
||||
updatedBy: string,
|
||||
policyId: string,
|
||||
operation: 'created' | 'updated' | 'deleted',
|
||||
rulesAfter: readonly PolicyRule[],
|
||||
rulesBefore: readonly PolicyRule[] | null,
|
||||
) {
|
||||
this.telemetry.track(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_UPDATED_NODE_TYPE_POLICY_DOCUMENT,
|
||||
{
|
||||
...policyActor(updatedBy),
|
||||
operation,
|
||||
policy_id: policyId,
|
||||
...countRuleActions(rulesAfter),
|
||||
previous_rule_count: rulesBefore?.length ?? null,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
private nodeTypePolicyAttachmentsUpdated({
|
||||
updatedBy,
|
||||
projectId,
|
||||
scopeId,
|
||||
before,
|
||||
after,
|
||||
}: RelayEventMap['node-type-policy-attachments-updated']) {
|
||||
this.telemetry.track(
|
||||
TELEMETRY_EVENT.NODE_TYPE_POLICIES.USER_UPDATED_NODE_TYPE_POLICY_ATTACHMENTS,
|
||||
{
|
||||
...policyActor(updatedBy),
|
||||
...policyScope(projectId),
|
||||
scope_id: scopeId,
|
||||
attachment_count: after.attachments.length,
|
||||
floor_attachment_count: after.attachments.filter((a) => a.isFloor).length,
|
||||
previous_attachment_count: before.attachments.length,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
// #endregion
|
||||
|
||||
// #region External secrets
|
||||
|
||||
private externalSecretsProviderSettingsSaved({
|
||||
|
|
|
|||
|
|
@ -1,10 +1,12 @@
|
|||
import type { Logger } from '@n8n/backend-common';
|
||||
import type { EngineConfig } from '@n8n/config';
|
||||
import { mintActionToken, mintIdentityToken, type LifecycleEvent } from '@n8n/engine';
|
||||
import type { Request, Response } from 'express';
|
||||
import request from 'supertest';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { EngineControlPlaneServer } from '../engine-control-plane-server';
|
||||
import type { EngineCredentialsController } from '../engine-credentials.controller';
|
||||
import type { EngineLifecycleEventPushRelay } from '../engine-lifecycle-event-push-relay';
|
||||
import { EngineLifecycleEventController } from '../engine-lifecycle-event.controller';
|
||||
|
||||
|
|
@ -24,6 +26,7 @@ describe('EngineControlPlaneServer', () => {
|
|||
let server: EngineControlPlaneServer;
|
||||
let serverLogger: Logger;
|
||||
let pushRelay: EngineLifecycleEventPushRelay;
|
||||
let credentialsController: EngineCredentialsController;
|
||||
let baseUrl: string;
|
||||
|
||||
const engineConfig = (overrides: Partial<EngineConfig> = {}) =>
|
||||
|
|
@ -38,10 +41,12 @@ describe('EngineControlPlaneServer', () => {
|
|||
beforeEach(async () => {
|
||||
serverLogger = mock<Logger>();
|
||||
pushRelay = mock<EngineLifecycleEventPushRelay>();
|
||||
credentialsController = mock<EngineCredentialsController>();
|
||||
const controller = new EngineLifecycleEventController(pushRelay);
|
||||
server = new EngineControlPlaneServer(
|
||||
engineConfig(),
|
||||
controller,
|
||||
credentialsController,
|
||||
mock<Logger>({ scoped: vi.fn().mockReturnValue(serverLogger) }),
|
||||
);
|
||||
await server.start();
|
||||
|
|
@ -59,6 +64,12 @@ describe('EngineControlPlaneServer', () => {
|
|||
return req.send(body as object);
|
||||
};
|
||||
|
||||
const postResolve = (body: unknown, token?: string) => {
|
||||
const req = request(baseUrl).post('/internal/credentials/resolve');
|
||||
if (token) req.set('Authorization', `Bearer ${token}`);
|
||||
return req.send(body as object);
|
||||
};
|
||||
|
||||
it('logs the port it actually bound, not the configured one', () => {
|
||||
// Configured as `0`, so the OS picked it: logging the configured value
|
||||
// would tell an operator to dial port 0.
|
||||
|
|
@ -135,6 +146,42 @@ describe('EngineControlPlaneServer', () => {
|
|||
expect(response.status).toBe(401);
|
||||
});
|
||||
|
||||
describe('credential resolve route', () => {
|
||||
const body = { credential: { id: 'cred-1' } };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(credentialsController.resolveCredential).mockImplementation(
|
||||
async (_req: Request, res: Response) => {
|
||||
res.status(200).json({ data: {} });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('hands an authenticated request with its parsed body to the controller', async () => {
|
||||
const response = await postResolve(body, mintActionToken(authSecret, 'credentials:read'));
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(credentialsController.resolveCredential).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.objectContaining({ body }),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['no token', undefined],
|
||||
['a lifecycle token', mintActionToken(authSecret, 'lifecycle-events:write')],
|
||||
[
|
||||
'a token signed with a different secret',
|
||||
mintActionToken('b'.repeat(32), 'credentials:read'),
|
||||
],
|
||||
])('rejects %s before the controller sees it', async (_label, token) => {
|
||||
const response = await postResolve(body, token);
|
||||
|
||||
expect(response.status).toBe(401);
|
||||
expect(credentialsController.resolveCredential).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it('stops listening when stopped', async () => {
|
||||
await server.stop();
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,86 @@
|
|||
import type { Request, Response } from 'express';
|
||||
import type { Mocked } from 'vitest';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
|
||||
|
||||
import type { ResolveCredentialRequest } from '../engine-credentials.contract';
|
||||
import { EngineCredentialsController } from '../engine-credentials.controller';
|
||||
import type { EngineCredentialsService } from '../engine-credentials.service';
|
||||
|
||||
const resolveRequest: ResolveCredentialRequest = {
|
||||
credential: { id: 'cred-1', name: 'Acme API', type: 'httpHeaderAuth' },
|
||||
execution: { executionId: 'exec-1', workflowId: 'wf-1', mode: 'manual' },
|
||||
context: { userId: 'user-1', projectId: 'project-1' },
|
||||
consumer: { nodeType: 'n8n-nodes-base.httpRequest' },
|
||||
};
|
||||
|
||||
const decrypted = { name: 'X-Api-Key', value: 'secret' };
|
||||
|
||||
describe('EngineCredentialsController', () => {
|
||||
let credentialsService: EngineCredentialsService;
|
||||
let controller: EngineCredentialsController;
|
||||
|
||||
const newResponse = () => {
|
||||
const res = { status: vi.fn(), json: vi.fn() };
|
||||
res.status.mockReturnValue(res);
|
||||
return res as unknown as Mocked<Response>;
|
||||
};
|
||||
|
||||
const newRequest = (body: unknown = resolveRequest) => ({ body }) as unknown as Request;
|
||||
|
||||
beforeEach(() => {
|
||||
credentialsService = mock<EngineCredentialsService>();
|
||||
controller = new EngineCredentialsController(credentialsService);
|
||||
|
||||
vi.mocked(credentialsService.resolve).mockResolvedValue(decrypted);
|
||||
});
|
||||
|
||||
describe('resolveCredential', () => {
|
||||
it('answers 200 with the decrypted data', async () => {
|
||||
const res = newResponse();
|
||||
|
||||
await controller.resolveCredential(newRequest(), res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledExactlyOnceWith(200);
|
||||
expect(res.json).toHaveBeenCalledExactlyOnceWith({ data: decrypted });
|
||||
});
|
||||
|
||||
it('hands the validated body to the service', async () => {
|
||||
await controller.resolveCredential(newRequest(), newResponse());
|
||||
|
||||
expect(credentialsService.resolve).toHaveBeenCalledExactlyOnceWith(resolveRequest);
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
'a body without a credential id',
|
||||
{ ...resolveRequest, credential: { name: 'x', type: 'y' } },
|
||||
],
|
||||
[
|
||||
'a body with an unknown mode',
|
||||
{ ...resolveRequest, execution: { ...resolveRequest.execution, mode: 'nope' } },
|
||||
],
|
||||
['a body that is not a request at all', { hello: 'world' }],
|
||||
])('rejects %s without a reason', async (_label, body) => {
|
||||
const res = newResponse();
|
||||
|
||||
await expect(controller.resolveCredential(newRequest(body), res)).rejects.toThrow(
|
||||
BadRequestError,
|
||||
);
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
// Unvalidated input must never reach the access check or the store.
|
||||
expect(credentialsService.resolve).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('passes a service error on without writing a response', async () => {
|
||||
const error = new ForbiddenError('Credential is not shared with the workflow');
|
||||
vi.mocked(credentialsService.resolve).mockRejectedValue(error);
|
||||
const res = newResponse();
|
||||
|
||||
await expect(controller.resolveCredential(newRequest(), res)).rejects.toBe(error);
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,121 @@
|
|||
import type { Logger } from '@n8n/backend-common';
|
||||
import type { IWorkflowExecuteAdditionalData } from 'n8n-workflow';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import type { CredentialsHelper } from '@/credentials-helper';
|
||||
import { CredentialNotFoundError } from '@/errors/credential-not-found.error';
|
||||
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
import type { CredentialsPermissionChecker } from '@/executions/pre-execution-checks';
|
||||
|
||||
import type { ResolveCredentialRequest } from '../engine-credentials.contract';
|
||||
import { EngineCredentialsService } from '../engine-credentials.service';
|
||||
|
||||
const mocks = vi.hoisted(() => ({ getBase: vi.fn() }));
|
||||
vi.mock('@/workflow-execute-additional-data', () => ({ getBase: mocks.getBase }));
|
||||
|
||||
const resolveRequest: ResolveCredentialRequest = {
|
||||
credential: { id: 'cred-1', name: 'Acme API', type: 'httpHeaderAuth' },
|
||||
execution: { executionId: 'exec-1', workflowId: 'wf-1', mode: 'manual' },
|
||||
context: { userId: 'user-1', projectId: 'project-1' },
|
||||
consumer: { nodeType: 'n8n-nodes-base.httpRequest' },
|
||||
};
|
||||
|
||||
const decrypted = { name: 'X-Api-Key', value: 'secret' };
|
||||
|
||||
describe('EngineCredentialsService', () => {
|
||||
let permissionChecker: CredentialsPermissionChecker;
|
||||
let credentialsHelper: CredentialsHelper;
|
||||
let logger: Logger;
|
||||
let service: EngineCredentialsService;
|
||||
let additionalData: IWorkflowExecuteAdditionalData;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
permissionChecker = mock<CredentialsPermissionChecker>();
|
||||
credentialsHelper = mock<CredentialsHelper>();
|
||||
logger = mock<Logger>();
|
||||
service = new EngineCredentialsService(
|
||||
permissionChecker,
|
||||
credentialsHelper,
|
||||
mock<Logger>({ scoped: vi.fn().mockReturnValue(logger) }),
|
||||
);
|
||||
|
||||
additionalData = {} as IWorkflowExecuteAdditionalData;
|
||||
mocks.getBase.mockResolvedValue(additionalData);
|
||||
vi.mocked(permissionChecker.findInaccessible).mockResolvedValue({
|
||||
homeProject: mock(),
|
||||
inaccessibleIds: [],
|
||||
});
|
||||
vi.mocked(credentialsHelper.getDecrypted).mockResolvedValue(decrypted);
|
||||
});
|
||||
|
||||
describe('resolve', () => {
|
||||
it('returns the decrypted data', async () => {
|
||||
await expect(service.resolve(resolveRequest)).resolves.toBe(decrypted);
|
||||
});
|
||||
|
||||
it('decrypts with the credential, mode and consumer node type from the request', async () => {
|
||||
await service.resolve(resolveRequest);
|
||||
|
||||
expect(credentialsHelper.getDecrypted).toHaveBeenCalledExactlyOnceWith(
|
||||
additionalData,
|
||||
{ id: 'cred-1', name: 'Acme API' },
|
||||
'httpHeaderAuth',
|
||||
'manual',
|
||||
expect.objectContaining({
|
||||
node: expect.objectContaining({ type: 'n8n-nodes-base.httpRequest' }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('builds additional data for the execution in the request', async () => {
|
||||
await service.resolve(resolveRequest);
|
||||
|
||||
expect(mocks.getBase).toHaveBeenCalledExactlyOnceWith({
|
||||
userId: 'user-1',
|
||||
workflowId: 'wf-1',
|
||||
projectId: 'project-1',
|
||||
});
|
||||
// The engine's id, so `$execution.id` in a credential reads the right one.
|
||||
expect(additionalData.executionId).toBe('exec-1');
|
||||
});
|
||||
|
||||
it('checks that the workflow may use the credential before decrypting', async () => {
|
||||
await service.resolve(resolveRequest);
|
||||
|
||||
expect(permissionChecker.findInaccessible).toHaveBeenCalledExactlyOnceWith('wf-1', [
|
||||
'cred-1',
|
||||
]);
|
||||
});
|
||||
|
||||
it('refuses a credential the workflow may not use, and logs it', async () => {
|
||||
vi.mocked(permissionChecker.findInaccessible).mockResolvedValue({
|
||||
homeProject: mock(),
|
||||
inaccessibleIds: ['cred-1'],
|
||||
});
|
||||
|
||||
await expect(service.resolve(resolveRequest)).rejects.toThrow(ForbiddenError);
|
||||
expect(credentialsHelper.getDecrypted).not.toHaveBeenCalled();
|
||||
// The client discards the body, so the operator's record is the log.
|
||||
expect(logger.warn).toHaveBeenCalledExactlyOnceWith(
|
||||
expect.stringContaining('Refused credential "cred-1" to workflow "wf-1"'),
|
||||
);
|
||||
});
|
||||
|
||||
it('reports not found when the store has no credential with that id and type', async () => {
|
||||
vi.mocked(credentialsHelper.getDecrypted).mockRejectedValue(
|
||||
new CredentialNotFoundError('cred-1', 'httpHeaderAuth'),
|
||||
);
|
||||
|
||||
await expect(service.resolve(resolveRequest)).rejects.toThrow(NotFoundError);
|
||||
});
|
||||
|
||||
it('passes on any other decryption error unchanged', async () => {
|
||||
const error = new Error('cipher unavailable');
|
||||
vi.mocked(credentialsHelper.getDecrypted).mockRejectedValue(error);
|
||||
|
||||
await expect(service.resolve(resolveRequest)).rejects.toBe(error);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -9,12 +9,15 @@ import { bodyParser, rawBodyReader } from '@/middlewares';
|
|||
import { send } from '@/response-helper';
|
||||
|
||||
import { createEngineControlPlaneAuthMiddleware } from './engine-control-plane-auth.middleware';
|
||||
import { STATUS_CALLBACK_PATH } from './engine-v2.constants';
|
||||
import { EngineCredentialsController } from './engine-credentials.controller';
|
||||
import { EngineLifecycleEventController } from './engine-lifecycle-event.controller';
|
||||
import { CREDENTIALS_RESOLVE_PATH, STATUS_CALLBACK_PATH } from './engine-v2.constants';
|
||||
|
||||
/**
|
||||
* Receives lifecycle events from the data plane. Its own server, not a route on
|
||||
* n8n's main one, so this surface can be isolated from the editor API.
|
||||
* Receives lifecycle events from the data plane and resolves credentials for
|
||||
* it. It listens on its own host and port, separate from the REST API server,
|
||||
* so an operator can restrict these internal routes to the network that the
|
||||
* data plane uses without touching the editor's routes.
|
||||
*/
|
||||
@Service()
|
||||
export class EngineControlPlaneServer {
|
||||
|
|
@ -28,6 +31,7 @@ export class EngineControlPlaneServer {
|
|||
constructor(
|
||||
private readonly engineConfig: EngineConfig,
|
||||
private readonly lifecycleEventController: EngineLifecycleEventController,
|
||||
private readonly credentialsController: EngineCredentialsController,
|
||||
private readonly logger: Logger,
|
||||
) {
|
||||
this.logger = this.logger.scoped('engine-v2');
|
||||
|
|
@ -102,5 +106,13 @@ export class EngineControlPlaneServer {
|
|||
async (req, res) => await this.lifecycleEventController.receiveLifecycleEvents(req, res),
|
||||
),
|
||||
);
|
||||
|
||||
app.post(
|
||||
CREDENTIALS_RESOLVE_PATH,
|
||||
createEngineControlPlaneAuthMiddleware(this.engineConfig, this.logger, 'credentials:read'),
|
||||
rawBodyReader,
|
||||
bodyParser,
|
||||
send(async (req, res) => await this.credentialsController.resolveCredential(req, res)),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,29 @@
|
|||
import { Service } from '@n8n/di';
|
||||
import type { Request, Response } from 'express';
|
||||
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
|
||||
import type { ResolveCredentialResponse } from './engine-credentials.contract';
|
||||
import { resolveCredentialRequestSchema } from './engine-credentials.contract';
|
||||
import { EngineCredentialsService } from './engine-credentials.service';
|
||||
|
||||
/**
|
||||
* Serves `POST /internal/credentials/resolve` for the engine 2.0 data plane.
|
||||
* Validates the body and writes the response. `EngineCredentialsService`
|
||||
* decides access and decrypts.
|
||||
*/
|
||||
@Service()
|
||||
export class EngineCredentialsController {
|
||||
constructor(private readonly credentialsService: EngineCredentialsService) {}
|
||||
|
||||
async resolveCredential(req: Request, res: Response): Promise<void> {
|
||||
const parsed = resolveCredentialRequestSchema.safeParse(req.body);
|
||||
|
||||
if (!parsed.success) throw new BadRequestError('Invalid credential resolve request');
|
||||
|
||||
const data = await this.credentialsService.resolve(parsed.data);
|
||||
|
||||
const body: ResolveCredentialResponse = { data };
|
||||
res.status(200).json(body);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,93 @@
|
|||
import { Logger } from '@n8n/backend-common';
|
||||
import { Service } from '@n8n/di';
|
||||
import type { ICredentialDataDecryptedObject, IExecuteData } from 'n8n-workflow';
|
||||
|
||||
import { CredentialsHelper } from '@/credentials-helper';
|
||||
import { CredentialNotFoundError } from '@/errors/credential-not-found.error';
|
||||
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks';
|
||||
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
|
||||
|
||||
import type { ResolveCredentialRequest } from './engine-credentials.contract';
|
||||
|
||||
/**
|
||||
* `getDecrypted` reads the consumer node type for the policy check from
|
||||
* `executeData.node.type`. The data plane sends the type only, so the rest of
|
||||
* the node is a placeholder. Nothing else in `getDecrypted` reads it: the node
|
||||
* has no parameters, so credential expressions resolve as they do with no
|
||||
* `executeData` at all.
|
||||
*/
|
||||
function toExecuteData(nodeType: string): IExecuteData {
|
||||
return {
|
||||
node: { id: '', name: '', type: nodeType, typeVersion: 0, position: [0, 0], parameters: {} },
|
||||
data: {},
|
||||
source: null,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a credential for the engine 2.0 data plane. The data plane has no
|
||||
* credential store and no encryption key, so this is the only place where a
|
||||
* step's credential is decrypted.
|
||||
*/
|
||||
@Service()
|
||||
export class EngineCredentialsService {
|
||||
constructor(
|
||||
private readonly permissionChecker: CredentialsPermissionChecker,
|
||||
private readonly credentialsHelper: CredentialsHelper,
|
||||
private readonly logger: Logger,
|
||||
) {
|
||||
this.logger = this.logger.scoped('engine-v2');
|
||||
}
|
||||
|
||||
/**
|
||||
* Throws `ForbiddenError` when the credential is not shared with one of the
|
||||
* workflow's projects, and `NotFoundError` when no credential has that id
|
||||
* and type. The access check runs before the row is loaded, so an unknown
|
||||
* id is usually refused as forbidden. The one exception is a workflow in the
|
||||
* personal project of a user with global credential access: the check
|
||||
* grants every id, so an unknown id is reported as not found.
|
||||
*/
|
||||
async resolve({
|
||||
credential,
|
||||
execution,
|
||||
context,
|
||||
consumer,
|
||||
}: ResolveCredentialRequest): Promise<ICredentialDataDecryptedObject> {
|
||||
// Checked on every request. The token proves the caller is the engine,
|
||||
// not that this workflow may use this credential. The dispatcher ran the
|
||||
// same check when the execution started, but a credential can be unshared
|
||||
// from the workflow's project while the execution is still running.
|
||||
const { inaccessibleIds } = await this.permissionChecker.findInaccessible(
|
||||
execution.workflowId,
|
||||
[credential.id],
|
||||
);
|
||||
if (inaccessibleIds.length > 0) {
|
||||
this.logger.warn(
|
||||
`Refused credential "${credential.id}" to workflow "${execution.workflowId}" - not shared with its projects`,
|
||||
);
|
||||
throw new ForbiddenError('Credential is not shared with the workflow');
|
||||
}
|
||||
|
||||
const additionalData = await WorkflowExecuteAdditionalData.getBase({
|
||||
userId: context.userId,
|
||||
workflowId: execution.workflowId,
|
||||
projectId: context.projectId,
|
||||
});
|
||||
additionalData.executionId = execution.executionId;
|
||||
|
||||
try {
|
||||
return await this.credentialsHelper.getDecrypted(
|
||||
additionalData,
|
||||
{ id: credential.id, name: credential.name },
|
||||
credential.type,
|
||||
execution.mode,
|
||||
toExecuteData(consumer.nodeType),
|
||||
);
|
||||
} catch (error) {
|
||||
if (error instanceof CredentialNotFoundError) throw new NotFoundError(error.message);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1334,5 +1334,44 @@ describe('create-workflow-from-code MCP tool', () => {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('top-level ceiling warning', () => {
|
||||
const wideNodes: INode[] = Array.from({ length: 8 }, (_, i) => ({
|
||||
id: `node-${i}`,
|
||||
name: `Step ${i}`,
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 1,
|
||||
position: [i * 200, 0],
|
||||
parameters: {},
|
||||
}));
|
||||
|
||||
test('flag on: a saved canvas over the ceiling with no groups gets a warning', async () => {
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
workflow: { ...mockWorkflowJson, nodes: wideNodes },
|
||||
warnings: [],
|
||||
});
|
||||
|
||||
const result = await callHandler(
|
||||
{ code: 'const wf = ...' },
|
||||
createTool({ canvasGroupsEnabled: true }),
|
||||
);
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(response.warnings).toEqual([
|
||||
expect.objectContaining({ code: 'TOP_LEVEL_ITEMS_OVER_CEILING' }),
|
||||
]);
|
||||
});
|
||||
|
||||
test('flag off: no ceiling warning, groups cannot be kept anyway', async () => {
|
||||
mockParseAndValidate.mockResolvedValue({
|
||||
workflow: { ...mockWorkflowJson, nodes: wideNodes },
|
||||
warnings: [],
|
||||
});
|
||||
|
||||
const result = await callHandler({ code: 'const wf = ...' }, createTool());
|
||||
|
||||
expect(parseResult(result)).not.toHaveProperty('warnings');
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,42 @@
|
|||
import type { IConnections, INode } from 'n8n-workflow';
|
||||
|
||||
import { topLevelItemsWarning } from '../tools/workflow-builder/top-level-items-warning';
|
||||
|
||||
const makeNodes = (count: number): INode[] =>
|
||||
Array.from({ length: count }, (_, i) => ({
|
||||
id: `n${i}`,
|
||||
name: `Node ${i}`,
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 1,
|
||||
position: [i * 200, 0],
|
||||
parameters: {},
|
||||
}));
|
||||
|
||||
describe('topLevelItemsWarning', () => {
|
||||
test('returns nothing when the canvas is within the ceiling', () => {
|
||||
expect(topLevelItemsWarning({ nodes: makeNodes(7), connections: {} })).toBeUndefined();
|
||||
});
|
||||
|
||||
test('returns one warning naming the loose nodes when the canvas is over the ceiling', () => {
|
||||
const warning = topLevelItemsWarning({ nodes: makeNodes(8), connections: {} });
|
||||
|
||||
expect(warning?.code).toBe('TOP_LEVEL_ITEMS_OVER_CEILING');
|
||||
expect(warning?.message).toContain('Node 0');
|
||||
});
|
||||
|
||||
test('counts a collapsed group as one box', () => {
|
||||
const nodes = makeNodes(8);
|
||||
const nodeGroups = [{ id: 'g1', name: 'Stage', nodeIds: nodes.slice(0, 4).map((n) => n.id) }];
|
||||
|
||||
expect(topLevelItemsWarning({ nodes, connections: {}, nodeGroups })).toBeUndefined();
|
||||
});
|
||||
|
||||
test('does not count a sub-node as a box', () => {
|
||||
const nodes = makeNodes(8);
|
||||
const connections: IConnections = {
|
||||
'Node 7': { ai_tool: [[{ node: 'Node 0', type: 'ai_tool', index: 0 }]] },
|
||||
};
|
||||
|
||||
expect(topLevelItemsWarning({ nodes, connections })).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
@ -1218,6 +1218,115 @@ describe('update-workflow MCP tool', () => {
|
|||
]),
|
||||
);
|
||||
});
|
||||
|
||||
describe('top-level ceiling warning', () => {
|
||||
const looseNodes = (count: number) =>
|
||||
Array.from({ length: count }, (_, i) =>
|
||||
makeNode({ id: `n${i}`, name: `Step ${i}`, position: [i * 200, 0] }),
|
||||
);
|
||||
const addNodeOps = (from: number, to: number) =>
|
||||
Array.from({ length: to - from }, (_, i) => ({
|
||||
type: 'addNode',
|
||||
node: makeNode({ id: `n${from + i}`, name: `Step ${from + i}` }),
|
||||
}));
|
||||
|
||||
test('an update that pushes the canvas over the ceiling gets a warning', async () => {
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(buildExistingWorkflow(), { nodes: looseNodes(6), connections: {} }),
|
||||
);
|
||||
|
||||
const result = await callHandler(
|
||||
{ workflowId: 'wf-1', operations: addNodeOps(6, 8) },
|
||||
createOnTool(),
|
||||
);
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(response.validationWarnings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ code: 'TOP_LEVEL_ITEMS_OVER_CEILING' }),
|
||||
]),
|
||||
);
|
||||
expect(response.validationWarnings).not.toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ preExisting: true })]),
|
||||
);
|
||||
});
|
||||
|
||||
test('a canvas already over the ceiling, with no box added, gets the warning marked pre-existing', async () => {
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(buildExistingWorkflow(), { nodes: looseNodes(9), connections: {} }),
|
||||
);
|
||||
|
||||
const result = await callHandler(
|
||||
{
|
||||
workflowId: 'wf-1',
|
||||
operations: [
|
||||
{
|
||||
type: 'updateNodeParameters',
|
||||
nodeName: 'Step 1',
|
||||
parameters: { url: 'https://new' },
|
||||
},
|
||||
],
|
||||
},
|
||||
createOnTool(),
|
||||
);
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(response.validationWarnings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
code: 'TOP_LEVEL_ITEMS_OVER_CEILING',
|
||||
preExisting: true,
|
||||
message: expect.stringContaining('[pre-existing]') as string,
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
test('swapping one loose node for a new one keeps the box count but is not pre-existing', async () => {
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(buildExistingWorkflow(), { nodes: looseNodes(9), connections: {} }),
|
||||
);
|
||||
|
||||
const result = await callHandler(
|
||||
{
|
||||
workflowId: 'wf-1',
|
||||
operations: [{ type: 'removeNode', nodeName: 'Step 8' }, ...addNodeOps(9, 10)],
|
||||
},
|
||||
createOnTool(),
|
||||
);
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(response.validationWarnings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ code: 'TOP_LEVEL_ITEMS_OVER_CEILING' }),
|
||||
]),
|
||||
);
|
||||
expect(response.validationWarnings).not.toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ preExisting: true })]),
|
||||
);
|
||||
});
|
||||
|
||||
test('a canvas already over the ceiling that this update adds loose nodes to is not marked pre-existing', async () => {
|
||||
findWorkflowMock.mockResolvedValue(
|
||||
Object.assign(buildExistingWorkflow(), { nodes: looseNodes(9), connections: {} }),
|
||||
);
|
||||
|
||||
const result = await callHandler(
|
||||
{ workflowId: 'wf-1', operations: addNodeOps(9, 11) },
|
||||
createOnTool(),
|
||||
);
|
||||
|
||||
const response = parseResult(result);
|
||||
expect(response.validationWarnings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ code: 'TOP_LEVEL_ITEMS_OVER_CEILING' }),
|
||||
]),
|
||||
);
|
||||
expect(response.validationWarnings).not.toEqual(
|
||||
expect.arrayContaining([expect.objectContaining({ preExisting: true })]),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import {
|
|||
} from './credentials-auto-assign';
|
||||
import { validateDataTableReferencesForWorkflow } from './data-table-validation';
|
||||
import { sanitizeSkillsUsed, SKILLS_USED_PARAM_DESCRIPTION } from './skills-used';
|
||||
import { topLevelItemsWarning } from './top-level-items-warning';
|
||||
import {
|
||||
buildCreateVersionMetadata,
|
||||
resolveVersionMetadata,
|
||||
|
|
@ -172,7 +173,9 @@ const outputSchema = {
|
|||
}),
|
||||
)
|
||||
.optional()
|
||||
.describe('Node groups that were invalid and skipped instead of failing the whole creation.'),
|
||||
.describe(
|
||||
'Node groups that were invalid and skipped instead of failing the whole creation. Repair them with update_workflow before you report the workflow as done.',
|
||||
),
|
||||
hint: z
|
||||
.string()
|
||||
.optional()
|
||||
|
|
@ -449,8 +452,14 @@ export const createCreateWorkflowFromCodeTool = (
|
|||
note: notes.length ? notes.join(' ') : undefined,
|
||||
skippedGroups: skippedGroups.length > 0 ? skippedGroups : undefined,
|
||||
};
|
||||
const output =
|
||||
result.warnings.length > 0 ? { ...baseOutput, warnings: result.warnings } : baseOutput;
|
||||
|
||||
// Groups are dropped on save when the flag is off, so only warn when they can be kept.
|
||||
const ceilingWarning = options.canvasGroupsEnabled
|
||||
? topLevelItemsWarning(savedWorkflow)
|
||||
: undefined;
|
||||
|
||||
const warnings = ceilingWarning ? [...result.warnings, ceilingWarning] : result.warnings;
|
||||
const output = warnings.length > 0 ? { ...baseOutput, warnings } : baseOutput;
|
||||
|
||||
return {
|
||||
content: [{ type: 'text', text: JSON.stringify(output, null, 2) }],
|
||||
|
|
|
|||
|
|
@ -65,7 +65,7 @@ export function getMcpInstructions(options: McpInstructionsOptions): string {
|
|||
const GROUPS_HINT = canvasGroupsEnabled
|
||||
? `
|
||||
|
||||
Node groups: when a workflow has several distinct stages, organise it into named groups so it is readable on the canvas. Before creating groups, call ${MCP_GET_SDK_REFERENCE_TOOL.toolName} with section "groups" for the rules, and ${MCP_GET_WORKFLOW_BEST_PRACTICES_TOOL.toolName} (technique "list") for when to group.`
|
||||
Node groups: when a workflow has several distinct stages, organise it into named groups so it is readable on the canvas. Before creating groups, call ${MCP_GET_SDK_REFERENCE_TOOL.toolName} with section "groups" for the rules, and ${MCP_GET_WORKFLOW_BEST_PRACTICES_TOOL.toolName} (technique "list") for when to group. The save never fails because of groups, so read its result: when it reports TOP_LEVEL_ITEMS_OVER_CEILING, skippedGroups or removedGroups, repair the groups with ${MCP_UPDATE_WORKFLOW_TOOL.toolName} before you tell the user the workflow is done. A warning marked [pre-existing] describes a canvas that was already like that before your update; you do not need to repair it before you report done.`
|
||||
: '';
|
||||
|
||||
const N8N_CONNECT_HINT = isN8nConnectAvailable
|
||||
|
|
|
|||
|
|
@ -0,0 +1,43 @@
|
|||
import type { IConnections, INode, IWorkflowGroup } from 'n8n-workflow';
|
||||
import {
|
||||
formatTopLevelItemsMessage,
|
||||
summarizeTopLevelItems,
|
||||
TOP_LEVEL_ITEMS_OVER_CEILING_CODE,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
export type TopLevelItemsWarning = { code: string; message: string };
|
||||
|
||||
type CanvasShape = {
|
||||
nodes: INode[];
|
||||
connections: IConnections;
|
||||
nodeGroups?: IWorkflowGroup[];
|
||||
};
|
||||
|
||||
const summarize = (workflow: CanvasShape) =>
|
||||
summarizeTopLevelItems({
|
||||
nodes: workflow.nodes,
|
||||
nodeGroups: workflow.nodeGroups,
|
||||
connectionsBySourceNode: workflow.connections,
|
||||
});
|
||||
|
||||
/** Names of the nodes that draw a box of their own: not in a group, not a sub-node. */
|
||||
export function summarizeUngroupedNodeNames(workflow: CanvasShape): string[] {
|
||||
return summarize(workflow).ungroupedNodeNames;
|
||||
}
|
||||
|
||||
/**
|
||||
* One warning when the saved canvas shows more boxes than the ceiling. A box is a
|
||||
* node or a collapsed group. Returns nothing when the canvas is within the ceiling.
|
||||
*/
|
||||
export function topLevelItemsWarning(workflow: CanvasShape): TopLevelItemsWarning | undefined {
|
||||
const summary = summarize(workflow);
|
||||
|
||||
if (!summary.overCeiling) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return {
|
||||
code: TOP_LEVEL_ITEMS_OVER_CEILING_CODE,
|
||||
message: formatTopLevelItemsMessage(summary),
|
||||
};
|
||||
}
|
||||
|
|
@ -34,6 +34,7 @@ import {
|
|||
} from './credentials-auto-assign';
|
||||
import { validateDataTableReferencesForUpdate } from './data-table-validation';
|
||||
import { sanitizeSkillsUsed, SKILLS_USED_PARAM_DESCRIPTION } from './skills-used';
|
||||
import { summarizeUngroupedNodeNames, topLevelItemsWarning } from './top-level-items-warning';
|
||||
import {
|
||||
buildUpdateVersionMetadata,
|
||||
resolveVersionMetadata,
|
||||
|
|
@ -350,7 +351,10 @@ const outputSchema = {
|
|||
reason: z.string(),
|
||||
}),
|
||||
)
|
||||
.optional(),
|
||||
.optional()
|
||||
.describe(
|
||||
'Existing groups this update made invalid and removed. Repair them before you report the workflow as done.',
|
||||
),
|
||||
settings: z
|
||||
.record(z.string(), z.unknown())
|
||||
.optional()
|
||||
|
|
@ -1348,6 +1352,35 @@ export const createUpdateWorkflowTool = (
|
|||
result.groupOperations,
|
||||
);
|
||||
|
||||
// Groups are dropped on save when the flag is off, so only warn when they can be kept.
|
||||
// A canvas that was already this wide before the update is marked pre-existing,
|
||||
// so the agent does not rework a layout it did not make.
|
||||
const ceilingWarning = canvasGroupsEnabled
|
||||
? topLevelItemsWarning(updatedWorkflow)
|
||||
: undefined;
|
||||
|
||||
if (ceilingWarning) {
|
||||
const preExistingUngroupedNodeNames = new Set(
|
||||
summarizeUngroupedNodeNames(existingWorkflow),
|
||||
);
|
||||
|
||||
const hasNewlyAddedUngroupedNodeNames = summarizeUngroupedNodeNames(updatedWorkflow).some(
|
||||
(name) => !preExistingUngroupedNodeNames.has(name),
|
||||
);
|
||||
|
||||
const wasOverCeilingBefore = topLevelItemsWarning(existingWorkflow) !== undefined;
|
||||
const preExisting = wasOverCeilingBefore && !hasNewlyAddedUngroupedNodeNames;
|
||||
validationWarnings.push(
|
||||
preExisting
|
||||
? {
|
||||
...ceilingWarning,
|
||||
message: `[pre-existing] ${ceilingWarning.message}`,
|
||||
preExisting: true,
|
||||
}
|
||||
: ceilingWarning,
|
||||
);
|
||||
}
|
||||
|
||||
const output: UpdateWorkflowOutput = {
|
||||
workflowId: updatedWorkflow.id,
|
||||
name: updatedWorkflow.name,
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
import { evaluateComposedType, evaluateType, type ScopePolicy } from '../policy-evaluator';
|
||||
import type { PolicyAttachment } from '../policy-rule.types';
|
||||
import {
|
||||
evaluateComposedType,
|
||||
evaluateType,
|
||||
partitionTypesByAction,
|
||||
type ScopePolicy,
|
||||
} from '../policy-evaluator';
|
||||
import type { PolicyAttachment, PolicyRule } from '../policy-rule.types';
|
||||
|
||||
const attachment = (overrides: Partial<PolicyAttachment>): PolicyAttachment => ({
|
||||
policyId: 'policy-1',
|
||||
|
|
@ -325,3 +330,66 @@ describe('evaluateComposedType', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('partitionTypesByAction', () => {
|
||||
const rule = (id: string, action: PolicyRule['action'], selector: PolicyRule['selector']) => ({
|
||||
id,
|
||||
action,
|
||||
selector,
|
||||
});
|
||||
|
||||
const TYPES = [
|
||||
'n8n-nodes-base.code',
|
||||
'n8n-nodes-base.executeCommand',
|
||||
'@acme/n8n-nodes-acme.thing',
|
||||
];
|
||||
|
||||
it('falls every type back to the default action when no rule matches', () => {
|
||||
expect(partitionTypesByAction([], 'deny', TYPES)).toEqual({
|
||||
allow: [],
|
||||
deny: TYPES,
|
||||
delegate: [],
|
||||
});
|
||||
});
|
||||
|
||||
it('expands a package selector across every type in that package', () => {
|
||||
const rules = [rule('r1', 'deny', { kind: 'package', value: 'n8n-nodes-base' })];
|
||||
|
||||
expect(partitionTypesByAction(rules, 'allow', TYPES)).toEqual({
|
||||
allow: ['@acme/n8n-nodes-acme.thing'],
|
||||
deny: ['n8n-nodes-base.code', 'n8n-nodes-base.executeCommand'],
|
||||
delegate: [],
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps first-match order, so an earlier name rule survives a later package rule', () => {
|
||||
const rules = [
|
||||
rule('r1', 'allow', { kind: 'name', value: 'n8n-nodes-base.code' }),
|
||||
rule('r2', 'deny', { kind: 'package', value: 'n8n-nodes-base' }),
|
||||
];
|
||||
|
||||
expect(partitionTypesByAction(rules, 'allow', TYPES)).toEqual({
|
||||
allow: ['n8n-nodes-base.code', '@acme/n8n-nodes-acme.thing'],
|
||||
deny: ['n8n-nodes-base.executeCommand'],
|
||||
delegate: [],
|
||||
});
|
||||
});
|
||||
|
||||
it('separates delegated types from allowed and denied ones', () => {
|
||||
const rules = [rule('r1', 'delegate', { kind: 'name', value: 'n8n-nodes-base.code' })];
|
||||
|
||||
expect(partitionTypesByAction(rules, 'allow', TYPES)).toEqual({
|
||||
allow: ['n8n-nodes-base.executeCommand', '@acme/n8n-nodes-acme.thing'],
|
||||
deny: [],
|
||||
delegate: ['n8n-nodes-base.code'],
|
||||
});
|
||||
});
|
||||
|
||||
it('returns empty buckets when the instance knows no types', () => {
|
||||
expect(partitionTypesByAction([], 'deny', [])).toEqual({
|
||||
allow: [],
|
||||
deny: [],
|
||||
delegate: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -242,6 +242,7 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
updatedBy: 'user-1',
|
||||
kind: KIND,
|
||||
policyId: created.id,
|
||||
origin: 'document-api',
|
||||
after: { rules: created.rules, version: created.version },
|
||||
});
|
||||
});
|
||||
|
|
@ -358,6 +359,7 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
updatedBy: 'user-2',
|
||||
kind: KIND,
|
||||
policyId: before.id,
|
||||
origin: 'document-api',
|
||||
before: { rules: [], version: 1 },
|
||||
after: { rules: [RULE], version: 2 },
|
||||
});
|
||||
|
|
@ -796,15 +798,26 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
[{ policyId: createdPolicy.id, priority: 0, isFloor: false }],
|
||||
ROOT,
|
||||
);
|
||||
expect(eventService.emit).toHaveBeenCalledTimes(2);
|
||||
expect(eventService.emit).toHaveBeenCalledTimes(3);
|
||||
expect(eventService.emit).toHaveBeenCalledWith(
|
||||
'node-type-policy-scope-updated',
|
||||
expect.objectContaining({ before: null }),
|
||||
);
|
||||
expect(eventService.emit).toHaveBeenCalledWith(
|
||||
'node-type-policy-document-created',
|
||||
expect.objectContaining({ policyId: createdPolicy.id }),
|
||||
expect.objectContaining({ policyId: createdPolicy.id, origin: 'composed-save' }),
|
||||
);
|
||||
expect(eventService.emit).toHaveBeenCalledWith('node-type-policy-saved', {
|
||||
updatedBy: 'user-1',
|
||||
kind: KIND,
|
||||
projectId: null,
|
||||
scopeId: createdScope.id,
|
||||
before: null,
|
||||
after: { defaultAction: 'deny', version: 2 },
|
||||
rulesBefore: null,
|
||||
rulesAfter: [RULE],
|
||||
warningCount: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('updates the existing scope and document, emitting both facets', async () => {
|
||||
|
|
@ -835,14 +848,26 @@ describe('TypeAvailabilityPolicyService', () => {
|
|||
'user-2',
|
||||
ROOT,
|
||||
);
|
||||
expect(eventService.emit).toHaveBeenCalledTimes(2);
|
||||
expect(eventService.emit).toHaveBeenCalledTimes(3);
|
||||
expect(eventService.emit).toHaveBeenCalledWith(
|
||||
'node-type-policy-document-updated',
|
||||
expect.objectContaining({
|
||||
origin: 'composed-save',
|
||||
before: { rules: [], version: 1 },
|
||||
after: { rules: [RULE], version: 2 },
|
||||
}),
|
||||
);
|
||||
expect(eventService.emit).toHaveBeenCalledWith('node-type-policy-saved', {
|
||||
updatedBy: 'user-2',
|
||||
kind: KIND,
|
||||
projectId: null,
|
||||
scopeId: scope.id,
|
||||
before: { defaultAction: 'allow', version: 1 },
|
||||
after: { defaultAction: 'deny', version: 3 },
|
||||
rulesBefore: [],
|
||||
rulesAfter: [RULE],
|
||||
warningCount: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('throws NotFoundError when the document update unexpectedly finds no row', async () => {
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import type { NodeTypeAvailabilityScope } from '@n8n/api-types';
|
|||
import type {
|
||||
PolicyAction,
|
||||
PolicyAttachment,
|
||||
PolicyRule,
|
||||
PolicySelector,
|
||||
PolicyVerdict,
|
||||
} from './policy-rule.types';
|
||||
|
|
@ -78,16 +79,54 @@ export function evaluateType(
|
|||
typeName: string,
|
||||
): PolicyVerdict {
|
||||
for (const attachment of orderedAttachments(attachments)) {
|
||||
for (const rule of attachment.rules) {
|
||||
if (selectorMatches(rule.selector, typeName)) {
|
||||
return { action: rule.action, matchedRuleId: rule.id };
|
||||
}
|
||||
}
|
||||
const matched = firstMatch(attachment.rules, typeName);
|
||||
if (matched) return matched;
|
||||
}
|
||||
|
||||
return { action: defaultAction, matchedRuleId: null };
|
||||
}
|
||||
|
||||
/** The first rule matching `typeName`, or `null` when none does. */
|
||||
function firstMatch(rules: readonly PolicyRule[], typeName: string): PolicyVerdict | null {
|
||||
for (const rule of rules) {
|
||||
if (selectorMatches(rule.selector, typeName)) {
|
||||
return { action: rule.action, matchedRuleId: rule.id };
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Same first-match evaluation as `evaluateType`, for a caller that already holds the flattened
|
||||
* rule sequence rather than the attachments it came from.
|
||||
*/
|
||||
export function evaluateRules(
|
||||
rules: readonly PolicyRule[],
|
||||
defaultAction: PolicyAction,
|
||||
typeName: string,
|
||||
): PolicyVerdict {
|
||||
return firstMatch(rules, typeName) ?? { action: defaultAction, matchedRuleId: null };
|
||||
}
|
||||
|
||||
/**
|
||||
* Sorts every known type name by what one scope's own policy decides for it. At project scope
|
||||
* this is the project's layer alone, before composition with the instance policy.
|
||||
*/
|
||||
export function partitionTypesByAction(
|
||||
rules: readonly PolicyRule[],
|
||||
defaultAction: PolicyAction,
|
||||
typeNames: readonly string[],
|
||||
): Record<PolicyAction, string[]> {
|
||||
const partition: Record<PolicyAction, string[]> = { allow: [], deny: [], delegate: [] };
|
||||
|
||||
for (const typeName of typeNames) {
|
||||
partition[evaluateRules(rules, defaultAction, typeName).action].push(typeName);
|
||||
}
|
||||
|
||||
return partition;
|
||||
}
|
||||
|
||||
/**
|
||||
* Composes an instance scope's verdict with a project scope's, per the finalized RFC:
|
||||
* conjunction with monotonic restriction. A project can only restrict further, never re-allow
|
||||
|
|
|
|||
|
|
@ -260,6 +260,7 @@ export class TypeAvailabilityPolicyService {
|
|||
updatedBy,
|
||||
kind,
|
||||
policyId: policy.id,
|
||||
origin: 'document-api',
|
||||
after: { rules: policy.rules, version: policy.version },
|
||||
});
|
||||
|
||||
|
|
@ -346,6 +347,7 @@ export class TypeAvailabilityPolicyService {
|
|||
updatedBy,
|
||||
kind: result.existing.kind,
|
||||
policyId,
|
||||
origin: 'document-api',
|
||||
before: { rules: result.existing.rules, version: result.existing.version },
|
||||
after: { rules: result.updated.rules, version: result.updated.version },
|
||||
});
|
||||
|
|
@ -662,6 +664,7 @@ export class TypeAvailabilityPolicyService {
|
|||
updatedBy,
|
||||
kind,
|
||||
policyId: result.policyId,
|
||||
origin: 'composed-save',
|
||||
after: result.documentAfter,
|
||||
});
|
||||
} else {
|
||||
|
|
@ -669,11 +672,24 @@ export class TypeAvailabilityPolicyService {
|
|||
updatedBy,
|
||||
kind,
|
||||
policyId: result.policyId,
|
||||
origin: 'composed-save',
|
||||
before: result.documentBefore ?? { rules: [], version: UNCONFIGURED_VERSION },
|
||||
after: result.documentAfter,
|
||||
});
|
||||
}
|
||||
|
||||
this.eventService.emit('node-type-policy-saved', {
|
||||
updatedBy,
|
||||
kind,
|
||||
projectId,
|
||||
scopeId: result.scopeId,
|
||||
before: result.scopeBefore,
|
||||
after: result.scopeAfter,
|
||||
rulesBefore: result.documentBefore?.rules ?? null,
|
||||
rulesAfter: result.documentAfter.rules,
|
||||
warningCount: warnings.length,
|
||||
});
|
||||
|
||||
return {
|
||||
scopeId: result.scopeId,
|
||||
defaultAction: result.scopeAfter.defaultAction,
|
||||
|
|
|
|||
|
|
@ -2,19 +2,54 @@
|
|||
|
||||
The frontend module contract and registries for n8n editor modules.
|
||||
|
||||
This package owns the `FrontendModuleDescription` descriptor type plus the modal
|
||||
and resource registries that features register against. The editor shell keeps
|
||||
the wiring (`moduleInitializer`) that drives the two-phase lifecycle; this
|
||||
package only defines the contract and the registry state.
|
||||
This package owns the `FrontendModuleDescription` descriptor type, the
|
||||
`defineFrontendModule()` helper that declares one, plus the modal and resource
|
||||
registries that features register against. The editor shell keeps the wiring
|
||||
(`moduleInitializer`) that drives the two-phase lifecycle; this package only
|
||||
defines the contract and the registry state.
|
||||
|
||||
It is a source-only package (no build step): consumers resolve it from `src`
|
||||
via the editor-ui Vite alias and `tsconfig` paths, and `src/index.ts` is the
|
||||
only public entry point.
|
||||
|
||||
```ts
|
||||
import { modalRegistry, registerResource, type FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule, modalRegistry, registerResource } from '@n8n/frontend-module-sdk';
|
||||
```
|
||||
|
||||
## Declaring a module
|
||||
|
||||
`defineFrontendModule()` is the canonical descriptor form. Use it for every
|
||||
module, in the shell and in a module package alike.
|
||||
|
||||
```ts
|
||||
export const MyFeatureModule = defineFrontendModule({
|
||||
// Must match the backend module id: both gate off `/rest/module-settings`.
|
||||
id: 'my-feature',
|
||||
name: 'My Feature',
|
||||
description: 'What this module does',
|
||||
icon: 'box',
|
||||
});
|
||||
```
|
||||
|
||||
The helper is the identity function at runtime. It returns the object that it
|
||||
gets. It does not wrap, clone, or freeze it, so a getter-backed field such as
|
||||
`available` stays lazy.
|
||||
|
||||
Two reasons to prefer it over a `: FrontendModuleDescription` annotation:
|
||||
|
||||
- **One seam.** The SDK gets one function to attach descriptor validation or a
|
||||
dev-mode check to, instead of declaration sites it cannot see.
|
||||
- **Inference.** `const T` keeps the literal type of each field, so
|
||||
`MyFeatureModule.id` is `'my-feature'` and not `string`.
|
||||
|
||||
Do not use both. An annotation on top of the helper widens the types again.
|
||||
|
||||
Keep the descriptor file import-light: types and the SDK at module scope only.
|
||||
Load a view with `async () => await import('./views/X.vue')`, and read a store
|
||||
inside a route guard, a push handler, or `setup`. The shell manifest imports
|
||||
every descriptor eagerly, so a top-level store import pulls the module into the
|
||||
boot chunk even when the module is off.
|
||||
|
||||
## Push handlers
|
||||
|
||||
A module owns a push message type when it declares it in `pushHandlers`. The
|
||||
|
|
|
|||
|
|
@ -0,0 +1,126 @@
|
|||
import { describe, it, expect, expectTypeOf } from 'vitest';
|
||||
|
||||
import { defineFrontendModule } from './defineFrontendModule';
|
||||
import type { FrontendModuleDescription } from './types/descriptor';
|
||||
|
||||
describe('defineFrontendModule', () => {
|
||||
it('should return the same object it was given', () => {
|
||||
const descriptor = {
|
||||
id: 'identity',
|
||||
name: 'Identity',
|
||||
description: 'A descriptor passed straight through',
|
||||
icon: 'box',
|
||||
};
|
||||
|
||||
expect(defineFrontendModule(descriptor)).toBe(descriptor);
|
||||
});
|
||||
|
||||
it('should not freeze, seal, or clone the descriptor', () => {
|
||||
const module: FrontendModuleDescription = defineFrontendModule({
|
||||
id: 'mutable',
|
||||
name: 'Mutable',
|
||||
description: 'A descriptor that stays writable',
|
||||
icon: 'box',
|
||||
resources: [{ key: 'mutable', displayName: 'Mutable' }],
|
||||
});
|
||||
|
||||
expect(Object.isFrozen(module)).toBe(false);
|
||||
expect(Object.isSealed(module)).toBe(false);
|
||||
expect(Object.isExtensible(module)).toBe(true);
|
||||
|
||||
// The shell reads a descriptor through the contract type, and registration
|
||||
// order lets a host append. Nothing the helper does may prevent that.
|
||||
module.resources?.push({ key: 'added', displayName: 'Added' });
|
||||
expect(module.resources).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('should keep a getter lazy instead of reading it', () => {
|
||||
let reads = 0;
|
||||
const module = defineFrontendModule({
|
||||
id: 'lazy',
|
||||
name: 'Lazy',
|
||||
description: 'A descriptor with a getter-backed field',
|
||||
icon: 'box',
|
||||
get settingsPages() {
|
||||
reads += 1;
|
||||
return [];
|
||||
},
|
||||
});
|
||||
|
||||
expect(reads).toBe(0);
|
||||
expect(module.settingsPages).toHaveLength(0);
|
||||
expect(reads).toBe(1);
|
||||
});
|
||||
|
||||
it('should keep the literal type of each field', () => {
|
||||
const module = defineFrontendModule({
|
||||
id: 'literal',
|
||||
name: 'Literal',
|
||||
description: 'A descriptor whose literal types survive',
|
||||
icon: 'box',
|
||||
});
|
||||
|
||||
expectTypeOf(module.id).toEqualTypeOf<'literal'>();
|
||||
expectTypeOf(module.name).toEqualTypeOf<'Literal'>();
|
||||
expectTypeOf(module.icon).toEqualTypeOf<'box'>();
|
||||
});
|
||||
|
||||
it('should narrow optional fields a descriptor declares', () => {
|
||||
const module = defineFrontendModule({
|
||||
id: 'narrow',
|
||||
name: 'Narrow',
|
||||
description: 'A descriptor declaring one optional surface',
|
||||
icon: 'box',
|
||||
commands: [{ id: 'narrow.open', title: 'Open Narrow' }],
|
||||
});
|
||||
|
||||
// An annotated descriptor widens this to `CommandBarEntry[] | undefined`.
|
||||
expectTypeOf(module.commands).not.toBeUndefined();
|
||||
expectTypeOf(module).toExtend<FrontendModuleDescription>();
|
||||
});
|
||||
|
||||
it('should type-check a descriptor against the contract', () => {
|
||||
// @ts-expect-error `id` is required.
|
||||
defineFrontendModule({ name: 'No id', description: '', icon: 'box' });
|
||||
|
||||
// @ts-expect-error `icon` must be a string.
|
||||
defineFrontendModule({ id: 'typed', name: 'Typed', description: '', icon: 42 });
|
||||
|
||||
// @ts-expect-error `routes` must hold route records.
|
||||
defineFrontendModule({ id: 'typed', name: 'Typed', description: '', icon: 'box', routes: [1] });
|
||||
});
|
||||
|
||||
it('should keep array element types narrow under `const` inference', () => {
|
||||
const module = defineFrontendModule({
|
||||
id: 'narrow-elements',
|
||||
name: 'Narrow elements',
|
||||
description: 'A descriptor whose array elements keep their literal types',
|
||||
icon: 'box',
|
||||
resources: [{ key: 'thing', displayName: 'Thing' }],
|
||||
});
|
||||
|
||||
expectTypeOf(module.resources[0].key).toEqualTypeOf<'thing'>();
|
||||
expect(module.resources[0].key).toBe('thing');
|
||||
});
|
||||
|
||||
it('should type a route guard from the contract', () => {
|
||||
const module = defineFrontendModule({
|
||||
id: 'guarded',
|
||||
name: 'Guarded',
|
||||
description: 'A descriptor whose guard parameter is typed by the contract',
|
||||
icon: 'box',
|
||||
routes: [
|
||||
{
|
||||
path: '/guarded',
|
||||
component: async () => await Promise.resolve({}),
|
||||
beforeEnter: (to) => {
|
||||
expectTypeOf(to.params).not.toBeAny();
|
||||
return true;
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(module.routes).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
import type { FrontendModuleDescription } from './types/descriptor';
|
||||
|
||||
/**
|
||||
* Declares a frontend module descriptor.
|
||||
*
|
||||
* Identity at runtime: it returns the object it was given. It does not wrap,
|
||||
* clone, or freeze it, so a descriptor declared through it behaves exactly as a
|
||||
* descriptor declared with a type annotation.
|
||||
*
|
||||
* What it adds is a seam and inference. The seam gives the SDK one place to
|
||||
* attach validation or a dev-mode check later, instead of ten declaration sites
|
||||
* it cannot see. The inference keeps each field's literal type — `MyModule.id`
|
||||
* reads as `'my-feature'`, not `string` — which an annotation widens away.
|
||||
*
|
||||
* Use it for every descriptor. An annotated descriptor still satisfies the type,
|
||||
* but it is not the canonical form.
|
||||
*
|
||||
* ```ts
|
||||
* export const MyFeatureModule = defineFrontendModule({
|
||||
* id: 'my-feature',
|
||||
* name: 'My Feature',
|
||||
* description: 'What this module does',
|
||||
* icon: 'box',
|
||||
* });
|
||||
* ```
|
||||
*/
|
||||
export function defineFrontendModule<const T extends FrontendModuleDescription>(module: T): T {
|
||||
return module;
|
||||
}
|
||||
|
|
@ -1,5 +1,6 @@
|
|||
export type * from './types';
|
||||
|
||||
export { defineFrontendModule } from './defineFrontendModule';
|
||||
export { assertUniqueRouteNames } from './routeNames';
|
||||
|
||||
export * as modalRegistry from './registries/modalRegistry';
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -11,7 +11,20 @@ import { InsightsModule } from '@n8n/frontend-module-insights/insights.module';
|
|||
import { PromotionsModule } from '@/features/integrations/promotions.ee/module.descriptor';
|
||||
|
||||
/**
|
||||
* Hard-coding modules list until we have a dynamic way to load modules.
|
||||
* The static list is the design, not a placeholder (design §9). n8n self-hosted
|
||||
* ships one artifact, and module availability is enforced by the backend — the
|
||||
* `licenseFlag` plus the `isModuleActive` route middleware — so a disabled
|
||||
* module's code on disk is inert bytes, not a leak. Fetching module bundles at
|
||||
* runtime would buy nothing and cost real failure modes: a chunk 404 after a
|
||||
* rolling deploy, a second vue/pinia instance, and boot ordering that races
|
||||
* `/rest/module-settings` against pre-mount route registration.
|
||||
*
|
||||
* Build-time per-module chunks are the open question, not runtime loading. Once
|
||||
* every descriptor is import-light, this array can become a static map of
|
||||
* dynamic imports and Vite emits one chunk per module. That is decided at
|
||||
* wave-2 exit, with bundle-analysis data.
|
||||
*
|
||||
* Add a module here through the scaffolder (`pnpm n8n-module-sdk create`).
|
||||
*/
|
||||
export const modules: FrontendModuleDescription[] = [
|
||||
DataTableModule,
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { VIEWS } from '@/app/constants';
|
||||
import { type FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
import {
|
||||
AGENTS_LIST_VIEW,
|
||||
AGENT_BUILDER_VIEW,
|
||||
|
|
@ -25,7 +25,7 @@ const AgentSessionsListView = async (): Promise<unknown> =>
|
|||
const AgentSessionTimelineView = async (): Promise<unknown> =>
|
||||
await import('@/features/agents/views/AgentSessionTimelineView.vue');
|
||||
|
||||
export const AgentsModule: FrontendModuleDescription = {
|
||||
export const AgentsModule = defineFrontendModule({
|
||||
id: 'agents',
|
||||
name: 'Agents',
|
||||
description: 'Build and manage AI agents',
|
||||
|
|
@ -122,4 +122,4 @@ export const AgentsModule: FrontendModuleDescription = {
|
|||
displayName: 'Agent',
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { type FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
import {
|
||||
CHAT_VIEW,
|
||||
CHAT_CONVERSATION_VIEW,
|
||||
|
|
@ -23,7 +23,7 @@ const ChatPersonalAgentsView = async () =>
|
|||
const SettingsChatHubView = async () =>
|
||||
await import('@/features/ai/chatHub/SettingsChatHubView.vue');
|
||||
|
||||
export const ChatModule: FrontendModuleDescription = {
|
||||
export const ChatModule = defineFrontendModule({
|
||||
id: 'chat-hub',
|
||||
name: 'Chat',
|
||||
description: 'Chat with LLM models or your n8n AI agents.',
|
||||
|
|
@ -205,4 +205,4 @@ export const ChatModule: FrontendModuleDescription = {
|
|||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { i18n } from '@n8n/i18n';
|
||||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
import { VIEWS } from '@/app/constants';
|
||||
import { INSTANCE_AI_MODALS } from './modals';
|
||||
import {
|
||||
|
|
@ -49,7 +49,7 @@ const InstanceAiEmptyView = async () => await import('./InstanceAiEmptyView.vue'
|
|||
const InstanceAiThreadView = async () => await import('./InstanceAiThreadView.vue');
|
||||
const SettingsInstanceAiView = async () => await import('./views/SettingsInstanceAiView.vue');
|
||||
|
||||
export const InstanceAiModule: FrontendModuleDescription = {
|
||||
export const InstanceAiModule = defineFrontendModule({
|
||||
id: 'instance-ai',
|
||||
name: 'n8n Assistant',
|
||||
description: 'Chat with your n8n instance.',
|
||||
|
|
@ -215,4 +215,4 @@ export const InstanceAiModule: FrontendModuleDescription = {
|
|||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { useI18n } from '@n8n/i18n';
|
||||
import { type FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
import { EXPOSE_ALL_WORKFLOWS_TO_MCP_MODALS } from '@/experiments/exposeAllWorkflowsToMcp/modals';
|
||||
import { SURFACE_MCP_TO_NEW_CLOUD_USERS_MODALS } from '@/experiments/surfaceMcpToNewCloudUsers/modals';
|
||||
import { MCP_JSON_NUDGE_MODALS } from '@/experiments/mcpJsonNudge/modals';
|
||||
|
|
@ -23,7 +23,7 @@ const SettingsMCPAgentsView = async () =>
|
|||
const SettingsMCPClientsView = async () =>
|
||||
await import('@/features/ai/mcpAccess/SettingsMCPClientsView.vue');
|
||||
|
||||
export const MCPModule: FrontendModuleDescription = {
|
||||
export const MCPModule = defineFrontendModule({
|
||||
id: 'mcp',
|
||||
name: 'MCP Server',
|
||||
description: 'Access your n8n instance through MCP clients',
|
||||
|
|
@ -107,4 +107,4 @@ export const MCPModule: FrontendModuleDescription = {
|
|||
...EXPOSE_ALL_WORKFLOWS_TO_MCP_MODALS,
|
||||
...MCP_JSON_NUDGE_MODALS,
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { screen, waitFor } from '@testing-library/vue';
|
|||
import userEvent from '@testing-library/user-event';
|
||||
import { reactive } from 'vue';
|
||||
import type { Component } from 'vue';
|
||||
import type { RouteRecordRaw } from 'vue-router';
|
||||
|
||||
import { createComponentRenderer } from '@/__tests__/render';
|
||||
import { defaultSettings, mockedStore } from '@n8n/frontend-test-utils';
|
||||
|
|
@ -50,7 +51,11 @@ const moduleSettings: FrontendModuleSettings = {
|
|||
|
||||
/** The dashboard as the shell reaches it: the descriptor's lazy route component. */
|
||||
const loadDashboard = async (): Promise<Component> => {
|
||||
const child = InsightsModule.routes?.[0]?.children?.[0];
|
||||
// Read the routes through the contract type. `defineFrontendModule()` keeps the
|
||||
// descriptor's literal types, so the inferred record narrows to the one route
|
||||
// shape insights declares — `RouteRecordRaw` is the shape the shell registers.
|
||||
const routes: RouteRecordRaw[] = InsightsModule.routes ?? [];
|
||||
const child = routes[0]?.children?.[0];
|
||||
const loader = child?.components?.default ?? child?.component;
|
||||
if (typeof loader !== 'function') throw new Error('insights dashboard route component missing');
|
||||
const loaded = (await (loader as () => Promise<{ default: Component }>)()) as {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { useI18n } from '@n8n/i18n';
|
||||
import { type FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
import {
|
||||
DATA_TABLE_DETAILS,
|
||||
DATA_TABLE_VIEW,
|
||||
|
|
@ -16,7 +16,7 @@ const DataTableView = async () => await import('@/features/core/dataTable/DataTa
|
|||
const DataTableDetailsView = async () =>
|
||||
await import('@/features/core/dataTable/DataTableDetailsView.vue');
|
||||
|
||||
export const DataTableModule: FrontendModuleDescription = {
|
||||
export const DataTableModule = defineFrontendModule({
|
||||
id: 'data-table',
|
||||
name: 'Data Table',
|
||||
description: 'Manage and store data efficiently with the Data Table module.',
|
||||
|
|
@ -95,4 +95,4 @@ export const DataTableModule: FrontendModuleDescription = {
|
|||
displayName: 'Data Table',
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
|
||||
import { VIEWS } from '@/app/constants';
|
||||
import { usePromotionsEnabled } from '@/features/shared/promotions/usePromotionsEnabled';
|
||||
|
|
@ -15,7 +15,7 @@ const SETTINGS_SCOPES = [
|
|||
'gitConnection:delete',
|
||||
] as const;
|
||||
|
||||
export const PromotionsModule: FrontendModuleDescription = {
|
||||
export const PromotionsModule = defineFrontendModule({
|
||||
id: 'promotions',
|
||||
name: 'Promotions',
|
||||
description: 'Promote workflow changes between environments',
|
||||
|
|
@ -41,4 +41,4 @@ export const PromotionsModule: FrontendModuleDescription = {
|
|||
},
|
||||
],
|
||||
modals: PROMOTIONS_MODALS,
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
|
||||
import { VIEWS } from '@/app/constants';
|
||||
|
||||
|
|
@ -8,7 +8,7 @@ import { WORKFLOW_REVIEW_REQUESTS_VIEW } from './constants';
|
|||
const WorkflowReviewRequestsView = async () =>
|
||||
await import('./views/WorkflowReviewRequestsView.vue');
|
||||
|
||||
export const WorkflowReviewsModule: FrontendModuleDescription = {
|
||||
export const WorkflowReviewsModule = defineFrontendModule({
|
||||
id: 'workflow-reviews',
|
||||
name: 'Workflow Reviews',
|
||||
description: 'Cross-project workflow review inbox.',
|
||||
|
|
@ -31,4 +31,4 @@ export const WorkflowReviewsModule: FrontendModuleDescription = {
|
|||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
import { VIEWS } from '@n8n/frontend-constants/views';
|
||||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
import { RouterView } from 'vue-router';
|
||||
|
||||
const InsightsDashboard = async () => await import('./components/InsightsDashboard.vue');
|
||||
|
||||
export const InsightsModule: FrontendModuleDescription = {
|
||||
export const InsightsModule = defineFrontendModule({
|
||||
id: 'insights',
|
||||
name: 'Insights',
|
||||
description: 'Provides insights and analytics features for projects.',
|
||||
|
|
@ -42,4 +42,4 @@ export const InsightsModule: FrontendModuleDescription = {
|
|||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
|
||||
/**
|
||||
* Store-only: this module contributes no UI surface, so registering it is a no-op —
|
||||
|
|
@ -6,10 +6,10 @@ import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
|||
* consume the store directly; the descriptor is what makes this a module the shell
|
||||
* knows about rather than a library it happens to import.
|
||||
*/
|
||||
export const InstanceRegistryModule: FrontendModuleDescription = {
|
||||
export const InstanceRegistryModule = defineFrontendModule({
|
||||
// Must match the backend module id: both gate off `/rest/module-settings`.
|
||||
id: 'instance-registry',
|
||||
name: 'Instance Registry',
|
||||
description: 'Reports which instances are in this deployment and their health',
|
||||
icon: 'server',
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk';
|
||||
import { defineFrontendModule } from '@n8n/frontend-module-sdk';
|
||||
import { useI18n } from '@n8n/i18n';
|
||||
import { useRBACStore } from '@n8n/stores/rbac.store';
|
||||
|
||||
|
|
@ -9,7 +9,7 @@ import { OTEL_SETTINGS_VIEW } from './otel.constants';
|
|||
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
|
||||
const SettingsOpenTelemetryView = async () => await import('./SettingsOpenTelemetryView.vue');
|
||||
|
||||
export const OtelModule: FrontendModuleDescription = {
|
||||
export const OtelModule = defineFrontendModule({
|
||||
id: 'otel',
|
||||
name: 'OpenTelemetry',
|
||||
description: 'Configure OpenTelemetry settings',
|
||||
|
|
@ -47,4 +47,4 @@ export const OtelModule: FrontendModuleDescription = {
|
|||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,140 @@
|
|||
import type { ILoadOptionsFunctions, INode } from 'n8n-workflow';
|
||||
import { NodeOperationError } from 'n8n-workflow';
|
||||
import type { MockedFunction } from 'vitest';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import { searchWorkbooks } from '../../../v2/methods/listSearch';
|
||||
import { getExcelCredentialType, microsoftApiRequest } from '../../../v2/transport';
|
||||
|
||||
vi.mock('../../../v2/transport', () => ({
|
||||
microsoftApiRequest: vi.fn(),
|
||||
getExcelCredentialType: vi.fn(),
|
||||
}));
|
||||
|
||||
const WORKBOOK = { id: 'wb1', name: 'Budget.xlsx', webUrl: 'https://c/wb1', file: {} };
|
||||
const MACRO_WORKBOOK = { id: 'wb2', name: 'Q4.XLSM', webUrl: 'https://c/wb2', file: {} };
|
||||
const DOUBLE_EXTENSION = {
|
||||
id: 'wb3',
|
||||
name: 'Report.xlsx.backup.xlsx',
|
||||
webUrl: 'https://c/wb3',
|
||||
file: {},
|
||||
};
|
||||
const SHORTCUT = { id: 'url1', name: 'SPBook.xlsx.url', webUrl: 'https://c/url1', file: {} };
|
||||
const FOLDER = {
|
||||
id: 'f1',
|
||||
name: 'Archive.xlsx',
|
||||
webUrl: 'https://c/f1',
|
||||
folder: { childCount: 2 },
|
||||
};
|
||||
|
||||
const DEFAULT_QUERY = "/drive/root/search(q='.xlsx OR .xlsm')";
|
||||
const DEFAULT_QS = { select: 'id,name,webUrl,file', $top: 100 };
|
||||
|
||||
describe('Microsoft Excel V2 - listSearch', () => {
|
||||
let ctx: ILoadOptionsFunctions;
|
||||
const apiRequest = microsoftApiRequest as MockedFunction<typeof microsoftApiRequest>;
|
||||
const credentialType = getExcelCredentialType as MockedFunction<typeof getExcelCredentialType>;
|
||||
|
||||
beforeEach(() => {
|
||||
apiRequest.mockReset();
|
||||
credentialType.mockReset();
|
||||
credentialType.mockReturnValue('microsoftOAuth2Api');
|
||||
ctx = mock<ILoadOptionsFunctions>({
|
||||
getNode: vi.fn(() => mock<INode>({ typeVersion: 2 })),
|
||||
});
|
||||
});
|
||||
|
||||
describe('searchWorkbooks', () => {
|
||||
it('lists only workbook files when no search text is given', async () => {
|
||||
apiRequest.mockResolvedValue({ value: [WORKBOOK, SHORTCUT, FOLDER] });
|
||||
|
||||
const result = await searchWorkbooks.call(ctx);
|
||||
|
||||
expect(apiRequest).toHaveBeenCalledWith(
|
||||
'GET',
|
||||
DEFAULT_QUERY,
|
||||
undefined,
|
||||
DEFAULT_QS,
|
||||
undefined,
|
||||
undefined,
|
||||
0,
|
||||
);
|
||||
expect(result).toEqual({
|
||||
results: [{ name: 'Budget', value: 'wb1', url: 'https://c/wb1' }],
|
||||
paginationToken: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it('applies the same workbook check to a user search, ignoring case', async () => {
|
||||
apiRequest.mockResolvedValue({ value: [MACRO_WORKBOOK, SHORTCUT] });
|
||||
|
||||
const result = await searchWorkbooks.call(ctx, 'q4');
|
||||
|
||||
expect(apiRequest).toHaveBeenCalledWith(
|
||||
'GET',
|
||||
"/drive/root/search(q='q4')",
|
||||
undefined,
|
||||
DEFAULT_QS,
|
||||
undefined,
|
||||
undefined,
|
||||
0,
|
||||
);
|
||||
expect(result.results).toEqual([{ name: 'Q4', value: 'wb2', url: 'https://c/wb2' }]);
|
||||
});
|
||||
|
||||
it('treats a blank search as no search text', async () => {
|
||||
apiRequest.mockResolvedValue({ value: [WORKBOOK] });
|
||||
|
||||
await searchWorkbooks.call(ctx, ' ');
|
||||
|
||||
expect(apiRequest).toHaveBeenCalledWith(
|
||||
'GET',
|
||||
DEFAULT_QUERY,
|
||||
undefined,
|
||||
DEFAULT_QS,
|
||||
undefined,
|
||||
undefined,
|
||||
0,
|
||||
);
|
||||
});
|
||||
|
||||
it('strips only the trailing extension from the display name', async () => {
|
||||
apiRequest.mockResolvedValue({ value: [DOUBLE_EXTENSION] });
|
||||
|
||||
const result = await searchWorkbooks.call(ctx);
|
||||
|
||||
expect(result.results).toEqual([
|
||||
{ name: 'Report.xlsx.backup', value: 'wb3', url: 'https://c/wb3' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('follows the pagination token and filters that page too', async () => {
|
||||
const token = 'https://graph.microsoft.com/v1.0/me/drive/root/search?$skiptoken=abc';
|
||||
const nextLink = 'https://graph.microsoft.com/v1.0/me/drive/root/search?$skiptoken=def';
|
||||
apiRequest.mockResolvedValue({ value: [SHORTCUT, WORKBOOK], '@odata.nextLink': nextLink });
|
||||
|
||||
const result = await searchWorkbooks.call(ctx, undefined, token);
|
||||
|
||||
expect(apiRequest).toHaveBeenCalledWith('GET', '', undefined, undefined, token, undefined, 0);
|
||||
expect(result).toEqual({
|
||||
results: [{ name: 'Budget', value: 'wb1', url: 'https://c/wb1' }],
|
||||
paginationToken: nextLink,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns no results when the response has no value collection', async () => {
|
||||
apiRequest.mockResolvedValue({});
|
||||
|
||||
const result = await searchWorkbooks.call(ctx);
|
||||
|
||||
expect(result.results).toEqual([]);
|
||||
});
|
||||
|
||||
it('rejects search with the Service Principal credential', async () => {
|
||||
credentialType.mockReturnValue('microsoftEntraServicePrincipalApi');
|
||||
|
||||
await expect(searchWorkbooks.call(ctx)).rejects.toThrow(NodeOperationError);
|
||||
expect(apiRequest).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -11,6 +11,26 @@ import { getExcelCredentialType, microsoftApiRequest } from '../transport';
|
|||
// listSearch context throughout this file: the transport's trailing `0` is its
|
||||
// fallback read (getNodeParameter's 2nd arg here is a fallback, not an item index).
|
||||
|
||||
const WORKBOOK_EXTENSIONS = ['.xlsx', '.xlsm'];
|
||||
|
||||
type DriveItem = {
|
||||
id?: string;
|
||||
name?: string;
|
||||
webUrl?: string;
|
||||
file?: IDataObject;
|
||||
};
|
||||
|
||||
type DriveSearchResponse = {
|
||||
value?: DriveItem[];
|
||||
'@odata.nextLink'?: string;
|
||||
};
|
||||
|
||||
function workbookExtension(item: DriveItem): string | undefined {
|
||||
if (item.file === undefined) return undefined;
|
||||
const name = (item.name ?? '').toLowerCase();
|
||||
return WORKBOOK_EXTENSIONS.find((extension) => name.endsWith(extension));
|
||||
}
|
||||
|
||||
export async function searchWorkbooks(
|
||||
this: ILoadOptionsFunctions,
|
||||
filter?: string,
|
||||
|
|
@ -27,61 +47,49 @@ export async function searchWorkbooks(
|
|||
},
|
||||
);
|
||||
}
|
||||
const fileExtensions = ['.xlsx', '.xlsm', '.xlst'];
|
||||
const extensionFilter = fileExtensions.join(' OR ');
|
||||
const trimmed = filter?.trim() ?? '';
|
||||
const q = trimmed === '' ? WORKBOOK_EXTENSIONS.join(' OR ') : trimmed;
|
||||
|
||||
const q = filter || extensionFilter;
|
||||
const response: DriveSearchResponse = paginationToken
|
||||
? await microsoftApiRequest.call(
|
||||
this,
|
||||
'GET',
|
||||
'',
|
||||
undefined,
|
||||
undefined,
|
||||
paginationToken, // paginationToken contains the full URL
|
||||
undefined,
|
||||
0,
|
||||
)
|
||||
: await microsoftApiRequest.call(
|
||||
this,
|
||||
'GET',
|
||||
`/drive/root/search(q='${q}')`,
|
||||
undefined,
|
||||
{
|
||||
select: 'id,name,webUrl,file',
|
||||
$top: 100,
|
||||
},
|
||||
undefined,
|
||||
undefined,
|
||||
0,
|
||||
);
|
||||
|
||||
let response: IDataObject = {};
|
||||
|
||||
if (paginationToken) {
|
||||
response = await microsoftApiRequest.call(
|
||||
this,
|
||||
'GET',
|
||||
'',
|
||||
undefined,
|
||||
undefined,
|
||||
paginationToken, // paginationToken contains the full URL
|
||||
undefined,
|
||||
0,
|
||||
);
|
||||
} else {
|
||||
response = await microsoftApiRequest.call(
|
||||
this,
|
||||
'GET',
|
||||
`/drive/root/search(q='${q}')`,
|
||||
undefined,
|
||||
{
|
||||
select: 'id,name,webUrl',
|
||||
$top: 100,
|
||||
},
|
||||
undefined,
|
||||
undefined,
|
||||
0,
|
||||
);
|
||||
}
|
||||
|
||||
if (response.value && filter) {
|
||||
response.value = (response.value as IDataObject[]).filter((workbook: IDataObject) => {
|
||||
return fileExtensions.some((extension) => (workbook.name as string).includes(extension));
|
||||
const results: INodeListSearchItems[] = [];
|
||||
for (const item of response.value ?? []) {
|
||||
const extension = workbookExtension(item);
|
||||
if (extension === undefined) continue;
|
||||
const name = item.name ?? '';
|
||||
results.push({
|
||||
name: name.slice(0, -extension.length),
|
||||
value: item.id ?? '',
|
||||
url: item.webUrl,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
results: (response.value as IDataObject[]).map((workbook: IDataObject) => {
|
||||
for (const extension of fileExtensions) {
|
||||
if ((workbook.name as string).includes(extension)) {
|
||||
workbook.name = (workbook.name as string).replace(extension, '');
|
||||
break;
|
||||
}
|
||||
}
|
||||
return {
|
||||
name: workbook.name as string,
|
||||
value: workbook.id as string,
|
||||
url: workbook.webUrl as string,
|
||||
};
|
||||
}),
|
||||
paginationToken: response['@odata.nextLink'] as string | undefined,
|
||||
results,
|
||||
paginationToken: response['@odata.nextLink'],
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ import {
|
|||
type IWorkflowGroup,
|
||||
type NodeConnectionType,
|
||||
} from './interfaces';
|
||||
import { isTriggerNode } from './node-helpers';
|
||||
import { isTriggerNode, isTriggerNodeType } from './node-helpers';
|
||||
|
||||
type NodeIo = NodeConnectionType | INodeInputConfiguration | INodeOutputConfiguration;
|
||||
type IODirection = 'inputs' | 'outputs';
|
||||
|
|
@ -41,6 +41,99 @@ export function normalizeGroupDescription(description: unknown): string | undefi
|
|||
return capped.length > 0 ? capped : undefined;
|
||||
}
|
||||
|
||||
/** Issue code shared by every surface that reports the collapsed box count. */
|
||||
export const TOP_LEVEL_ITEMS_OVER_CEILING_CODE = 'TOP_LEVEL_ITEMS_OVER_CEILING';
|
||||
|
||||
export type TopLevelItemsSummary = {
|
||||
/** Boxes on the canvas with every group collapsed: groups plus ungrouped nodes. */
|
||||
total: number;
|
||||
groupCount: number;
|
||||
ceiling: number;
|
||||
overCeiling: boolean;
|
||||
/** Ungrouped nodes that draw a box, the trigger included. */
|
||||
ungroupedNodeNames: string[];
|
||||
/** Ungrouped nodes a group could still hold; a trigger cannot join one. */
|
||||
groupableNodeNames: string[];
|
||||
};
|
||||
|
||||
type TopLevelItemsNode = Pick<INode, 'type'> & { id?: string; name?: string };
|
||||
|
||||
/**
|
||||
* Names of the nodes that reach their parent over non-main connections only. A
|
||||
* sub-node rides with its parent on the canvas, so it is not a box of its own.
|
||||
*/
|
||||
export function collectSubNodeNames(
|
||||
connectionsBySourceNode: IConnections | undefined,
|
||||
): Set<string> {
|
||||
const subNodeNames = new Set<string>();
|
||||
|
||||
for (const [nodeName, connectionsByType] of Object.entries(connectionsBySourceNode ?? {})) {
|
||||
// An empty slot (`ai_tool: [[]]`) is not a connection, so it must not make a sub-node.
|
||||
const connectedTypes = Object.entries(connectionsByType)
|
||||
.filter(([, outputs]) => outputs.some((targets) => (targets?.length ?? 0) > 0))
|
||||
.map(([type]) => type);
|
||||
|
||||
if (
|
||||
connectedTypes.length > 0 &&
|
||||
connectedTypes.every((type) => type !== NodeConnectionTypes.Main)
|
||||
) {
|
||||
subNodeNames.add(nodeName);
|
||||
}
|
||||
}
|
||||
|
||||
return subNodeNames;
|
||||
}
|
||||
|
||||
/**
|
||||
* Counts the boxes a reader sees with every group collapsed. Sticky notes and
|
||||
* sub-nodes do not count: a sticky belongs to the user, a sub-node rides with its
|
||||
* parent. Shared by the Instance AI build and the MCP save tools so both report
|
||||
* the same number the grouping guidance states.
|
||||
*/
|
||||
export function summarizeTopLevelItems(input: {
|
||||
nodes: TopLevelItemsNode[];
|
||||
nodeGroups?: Array<Pick<IWorkflowGroup, 'nodeIds'>>;
|
||||
connectionsBySourceNode?: IConnections;
|
||||
}): TopLevelItemsSummary {
|
||||
const groups = input.nodeGroups ?? [];
|
||||
const groupedNodeIds = new Set(groups.flatMap((group) => group.nodeIds));
|
||||
const subNodeNames = collectSubNodeNames(input.connectionsBySourceNode);
|
||||
|
||||
const ungrouped = input.nodes.filter((node) => {
|
||||
const isSticky = node.type === STICKY_NODE_TYPE;
|
||||
const isGrouped = node.id !== undefined && groupedNodeIds.has(node.id);
|
||||
const isSubNode = node.name !== undefined && subNodeNames.has(node.name);
|
||||
|
||||
return !isSticky && !isGrouped && !isSubNode;
|
||||
});
|
||||
|
||||
const labelOf = (node: TopLevelItemsNode) => node.name ?? node.id ?? node.type;
|
||||
const total = groups.length + ungrouped.length;
|
||||
|
||||
return {
|
||||
total,
|
||||
groupCount: groups.length,
|
||||
ceiling: TOP_LEVEL_ITEM_CEILING,
|
||||
overCeiling: total > TOP_LEVEL_ITEM_CEILING,
|
||||
ungroupedNodeNames: ungrouped.map(labelOf),
|
||||
groupableNodeNames: ungrouped.filter((node) => !isTriggerNodeType(node.type)).map(labelOf),
|
||||
};
|
||||
}
|
||||
|
||||
/** The over-ceiling message, worded once so every surface tells the agent the same thing. */
|
||||
export function formatTopLevelItemsMessage(summary: TopLevelItemsSummary): string {
|
||||
const stillUngrouped =
|
||||
summary.groupableNodeNames.length > 0
|
||||
? `. Still ungrouped: ${summary.groupableNodeNames.join(', ')}`
|
||||
: '';
|
||||
|
||||
return (
|
||||
`The canvas top level has ${summary.total} boxes with every group collapsed, over the ${summary.ceiling} you should aim for` +
|
||||
stillUngrouped +
|
||||
'. Group any stage that can form a valid group and build again, or say why each of them cannot join one.'
|
||||
);
|
||||
}
|
||||
|
||||
export type NodeGroupingValidationInput<TNode extends INode = INode> = {
|
||||
nodes: TNode[];
|
||||
connectionsBySourceNode: IConnections;
|
||||
|
|
|
|||
|
|
@ -1,8 +1,12 @@
|
|||
import {
|
||||
collectSubNodeNames,
|
||||
dropInvalidWorkflowGroups,
|
||||
formatTopLevelItemsMessage,
|
||||
GROUP_DESCRIPTION_MAX_LENGTH,
|
||||
makeGetNodeTypeForGrouping,
|
||||
normalizeGroupDescription,
|
||||
summarizeTopLevelItems,
|
||||
TOP_LEVEL_ITEM_CEILING,
|
||||
validateNodeSelectionForExtraction,
|
||||
validateNodeSelectionForGrouping,
|
||||
validateWorkflowGroups,
|
||||
|
|
@ -1117,3 +1121,119 @@ describe('dropInvalidWorkflowGroups', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('summarizeTopLevelItems', () => {
|
||||
const plainNodes = (count: number) =>
|
||||
Array.from({ length: count }, (_, i) => makeNode({ id: `n${i}`, name: `N${i}` }));
|
||||
|
||||
it('stays under the ceiling with exactly TOP_LEVEL_ITEM_CEILING boxes', () => {
|
||||
const summary = summarizeTopLevelItems({ nodes: plainNodes(TOP_LEVEL_ITEM_CEILING) });
|
||||
|
||||
expect(summary.total).toBe(TOP_LEVEL_ITEM_CEILING);
|
||||
expect(summary.overCeiling).toBe(false);
|
||||
});
|
||||
|
||||
it('counts the trigger as a box but leaves it out of the groupable list', () => {
|
||||
const nodes = [
|
||||
makeNode({ id: 't', name: 'When chat message received', type: 'n8n-nodes-base.chatTrigger' }),
|
||||
...plainNodes(TOP_LEVEL_ITEM_CEILING),
|
||||
];
|
||||
|
||||
const summary = summarizeTopLevelItems({ nodes });
|
||||
|
||||
expect(summary.total).toBe(TOP_LEVEL_ITEM_CEILING + 1);
|
||||
expect(summary.overCeiling).toBe(true);
|
||||
expect(summary.ungroupedNodeNames).toContain('When chat message received');
|
||||
expect(summary.groupableNodeNames).not.toContain('When chat message received');
|
||||
expect(summary.groupableNodeNames).toHaveLength(TOP_LEVEL_ITEM_CEILING);
|
||||
});
|
||||
|
||||
it('counts a group as one box and its members as none', () => {
|
||||
const summary = summarizeTopLevelItems({
|
||||
nodes: plainNodes(8),
|
||||
nodeGroups: [{ nodeIds: ['n0', 'n1', 'n2'] }],
|
||||
});
|
||||
|
||||
// 1 group + 5 ungrouped nodes.
|
||||
expect(summary.total).toBe(6);
|
||||
expect(summary.groupCount).toBe(1);
|
||||
expect(summary.groupableNodeNames).toEqual(['N3', 'N4', 'N5', 'N6', 'N7']);
|
||||
});
|
||||
|
||||
it('counts an agent and its sub-nodes as one box', () => {
|
||||
const connections: IConnections = {
|
||||
Model: { ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 0 }]] },
|
||||
Memory: { ai_memory: [[{ node: 'Agent', type: 'ai_memory', index: 0 }]] },
|
||||
Tool: { ai_tool: [[{ node: 'Agent', type: 'ai_tool', index: 0 }]] },
|
||||
Agent: { main: [[{ node: 'N0', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
};
|
||||
const nodes = [
|
||||
makeNode({ id: 'agent', name: 'Agent' }),
|
||||
makeNode({ id: 'model', name: 'Model' }),
|
||||
makeNode({ id: 'memory', name: 'Memory' }),
|
||||
makeNode({ id: 'tool', name: 'Tool' }),
|
||||
...plainNodes(6),
|
||||
];
|
||||
|
||||
const summary = summarizeTopLevelItems({ nodes, connectionsBySourceNode: connections });
|
||||
|
||||
// Agent + 6 plain nodes; the three sub-nodes do not count.
|
||||
expect(summary.total).toBe(7);
|
||||
expect(summary.overCeiling).toBe(false);
|
||||
});
|
||||
|
||||
it('does not count sticky notes', () => {
|
||||
const summary = summarizeTopLevelItems({
|
||||
nodes: [...plainNodes(TOP_LEVEL_ITEM_CEILING), makeStickyNode()],
|
||||
});
|
||||
|
||||
expect(summary.total).toBe(TOP_LEVEL_ITEM_CEILING);
|
||||
});
|
||||
|
||||
it('treats a node without an id as ungrouped', () => {
|
||||
const nodes = [
|
||||
{ type: 'n8n-nodes-base.set', name: 'Anonymous' },
|
||||
...plainNodes(TOP_LEVEL_ITEM_CEILING),
|
||||
];
|
||||
|
||||
const summary = summarizeTopLevelItems({ nodes, nodeGroups: [{ nodeIds: ['n0'] }] });
|
||||
|
||||
expect(summary.total).toBe(TOP_LEVEL_ITEM_CEILING + 1);
|
||||
expect(summary.groupableNodeNames).toContain('Anonymous');
|
||||
});
|
||||
|
||||
it('lists the groupable nodes in the message and leaves the trigger out', () => {
|
||||
const nodes = [
|
||||
makeNode({ id: 't', name: 'Start', type: 'n8n-nodes-base.manualTrigger' }),
|
||||
...plainNodes(TOP_LEVEL_ITEM_CEILING),
|
||||
];
|
||||
|
||||
const message = formatTopLevelItemsMessage(summarizeTopLevelItems({ nodes }));
|
||||
|
||||
expect(message).toContain(`${TOP_LEVEL_ITEM_CEILING + 1} boxes`);
|
||||
expect(message).toContain('Still ungrouped: N0, N1');
|
||||
expect(message).not.toContain('Start');
|
||||
});
|
||||
});
|
||||
|
||||
describe('collectSubNodeNames', () => {
|
||||
it('keeps a node with a main output out of the sub-node set', () => {
|
||||
const connections: IConnections = {
|
||||
Model: { ai_languageModel: [[{ node: 'Agent', type: 'ai_languageModel', index: 0 }]] },
|
||||
Agent: { main: [[{ node: 'Next', type: NodeConnectionTypes.Main, index: 0 }]] },
|
||||
};
|
||||
|
||||
expect([...collectSubNodeNames(connections)]).toEqual(['Model']);
|
||||
expect(collectSubNodeNames(undefined).size).toBe(0);
|
||||
});
|
||||
|
||||
it('does not treat a node with only empty non-main slots as a sub-node', () => {
|
||||
const connections: IConnections = {
|
||||
'Loose Tool': { ai_tool: [[]] },
|
||||
'Wired Tool': { ai_tool: [[{ node: 'Agent', type: 'ai_tool', index: 0 }]] },
|
||||
Agent: { main: [[]], ai_tool: [[{ node: 'Other Agent', type: 'ai_tool', index: 0 }]] },
|
||||
};
|
||||
|
||||
expect([...collectSubNodeNames(connections)].sort()).toEqual(['Agent', 'Wired Tool']);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user