fix(core): Make placeholder() return string (no-changelog) (#30100)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mutasem Aldmour 2026-05-11 13:32:35 +02:00 committed by GitHub
parent e3e70d6068
commit 0feec2fea6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 480 additions and 152 deletions

View File

@ -65,6 +65,7 @@ interface PairwiseArgs {
concurrency: number;
maxExamples?: number;
exampleIds?: Set<string>;
examplesJsonl?: string;
timeoutMs: number;
outputDir: string;
judgeModel: string;
@ -104,6 +105,7 @@ function parseArgs(argv: string[]): PairwiseArgs {
parsePositiveInt(get('--concurrency'), '--concurrency') ?? Number(DEFAULTS.CONCURRENCY),
maxExamples: parsePositiveInt(get('--max-examples'), '--max-examples'),
exampleIds,
examplesJsonl: get('--examples-jsonl'),
timeoutMs:
parsePositiveNumber(get('--timeout-ms'), '--timeout-ms') ?? Number(DEFAULTS.TIMEOUT_MS),
outputDir: get('--output-dir') ?? defaultOutputDir,
@ -180,6 +182,9 @@ interface DatasetExample {
}
async function loadExamples(args: PairwiseArgs, logger: EvalLogger): Promise<DatasetExample[]> {
if (args.examplesJsonl) {
return loadExamplesFromJsonl(args.examplesJsonl, logger);
}
logger.info(`Fetching dataset "${args.dataset}" from LangSmith`);
const lsClient = new LangSmithClient();
const examples: DatasetExample[] = [];
@ -217,6 +222,52 @@ async function loadExamples(args: PairwiseArgs, logger: EvalLogger): Promise<Dat
return examples;
}
/**
* Load examples from a JSONL file. Accepts the shape produced by a previous
* pairwise run (`results.jsonl`) where each row carries `exampleId`, `prompt`,
* `dos`, `donts`. Useful for re-running a frozen example set after the source
* LangSmith dataset has changed.
*/
function loadExamplesFromJsonl(filePath: string, logger: EvalLogger): DatasetExample[] {
const absolute = path.resolve(filePath);
logger.info(`Loading examples from local JSONL: ${absolute}`);
const content = readFileSync(absolute, 'utf8');
const examples: DatasetExample[] = [];
const seen = new Set<string>();
let row = 0;
for (const line of content.split('\n')) {
row++;
const trimmed = line.trim();
if (!trimmed) continue;
let parsed: unknown;
try {
parsed = JSON.parse(trimmed);
} catch (error) {
logger.warn(`Skipping JSONL row ${row}: invalid JSON (${(error as Error).message})`);
continue;
}
if (!isRecord(parsed)) continue;
const id = typeof parsed.exampleId === 'string' ? parsed.exampleId : '';
const prompt = typeof parsed.prompt === 'string' ? parsed.prompt : '';
if (!id || !prompt) {
logger.warn(`Skipping JSONL row ${row}: missing exampleId or prompt`);
continue;
}
// Each iteration of the same example yields a row in results.jsonl;
// dedupe by id so we only run the example once per requested iteration.
if (seen.has(id)) continue;
seen.add(id);
examples.push({
id,
prompt,
dos: typeof parsed.dos === 'string' ? parsed.dos : undefined,
donts: typeof parsed.donts === 'string' ? parsed.donts : undefined,
});
}
logger.info(`Loaded ${examples.length} unique examples from ${absolute}`);
return examples;
}
// ---------------------------------------------------------------------------
// Per-example runner
// ---------------------------------------------------------------------------

View File

@ -56,14 +56,7 @@ const createMockSDKFunctions = (): SDKFunctions => ({
content,
options,
})),
placeholder: jest.fn((value: string) => ({
__placeholder: true as const,
hint: value,
toString: () => `<__PLACEHOLDER_VALUE__${value}__>`,
toJSON() {
return this.toString();
},
})),
placeholder: jest.fn((value: string) => `<__PLACEHOLDER_VALUE__${value}__>`),
newCredential: jest.fn((name: string) => ({ __newCredential: true, name })),
ifElse: jest.fn(),
switchCase: jest.fn(),
@ -970,17 +963,15 @@ describe('AST Interpreter', () => {
});
});
describe('expr(placeholder(...)) error', () => {
it('should throw clear error when expr receives a PlaceholderValue', () => {
describe('expr(placeholder(...)) round-trip', () => {
it('prepends = to the placeholder marker so it parses as an n8n expression', () => {
const funcs: SDKFunctions = {
...createMockSDKFunctions(),
expr,
};
const code = `const val = expr(placeholder('Your ID'));
export default val;`;
expect(() => interpretSDKCode(code, funcs)).toThrow(
"expr(placeholder('Your ID')) is invalid",
);
expect(interpretSDKCode(code, funcs)).toBe('=<__PLACEHOLDER_VALUE__Your ID__>');
});
});
});

View File

@ -30,11 +30,9 @@ describe('Expression System', () => {
expect(result).toBe("={{ $('Config').item.json.apiUrl }}");
});
it('should throw clear error when called with a PlaceholderValue', () => {
const placeholderObj = { __placeholder: true, hint: 'Your API URL' };
expect(() => expr(placeholderObj as unknown as string)).toThrow(
"expr(placeholder('Your API URL')) is invalid. Use placeholder() directly as the value, not inside expr().",
);
it('prepends = to a placeholder marker (preserves round-trip with `=marker`)', () => {
const marker = '<__PLACEHOLDER_VALUE__Your API URL__>';
expect(expr(marker)).toBe('=' + marker);
});
it('should throw clear error when called with a NewCredentialValue', () => {

View File

@ -36,17 +36,6 @@ export function isExpression(value: unknown): boolean {
* expr('={{ $json.x }}') // '={{ $json.x }}' (strips redundant =)
* ```
*/
function isPlaceholderLike(value: unknown): value is { __placeholder: true; hint: string } {
return (
typeof value === 'object' &&
value !== null &&
'__placeholder' in value &&
(value as Record<string, unknown>).__placeholder === true &&
'hint' in value &&
typeof (value as Record<string, unknown>).hint === 'string'
);
}
function isNewCredentialLike(value: unknown): value is { __newCredential: true; name: string } {
return (
typeof value === 'object' &&
@ -60,15 +49,10 @@ function isNewCredentialLike(value: unknown): value is { __newCredential: true;
export function expr(expression: string): string {
if (typeof expression !== 'string') {
// At runtime, the AST interpreter may pass non-string values (e.g. PlaceholderImpl objects).
// At runtime, the AST interpreter may pass non-string values (e.g. NewCredentialImpl objects).
// TypeScript narrows to `never` here since the param is typed as `string`,
// so we re-bind as `unknown` to perform runtime type checks.
const value: unknown = expression;
if (isPlaceholderLike(value)) {
throw new Error(
`expr(placeholder('${value.hint}')) is invalid. Use placeholder() directly as the value, not inside expr().`,
);
}
if (isNewCredentialLike(value)) {
throw new Error(
`expr(newCredential('${value.name}')) is invalid. Use newCredential() directly in the credentials config, not inside expr().`,

View File

@ -365,7 +365,7 @@ describe('generate-types', () => {
it('should map string type with Expression wrapper', () => {
const prop: NodeProperty = { name: 'url', displayName: 'URL', type: 'string', default: '' };
const result = generateTypes.mapPropertyType(prop);
expect(result).toBe('string | Expression<string> | PlaceholderValue');
expect(result).toBe('string | Expression<string>');
});
it('should map number type with Expression wrapper', () => {
@ -554,7 +554,7 @@ describe('generate-types', () => {
},
};
const result = generateTypes.mapPropertyType(prop);
expect(result).toBe('Array<string | Expression<string> | PlaceholderValue>');
expect(result).toBe('Array<string | Expression<string>>');
});
it('should map fixedCollection type to proper nested interface', () => {
@ -606,7 +606,7 @@ describe('generate-types', () => {
],
};
const result = generateTypes.mapPropertyType(prop);
expect(result).toContain('attendees?: Array<string | Expression<string> | PlaceholderValue>');
expect(result).toContain('attendees?: Array<string | Expression<string>>');
});
it('should map fixedCollection with multipleValues to array type', () => {
@ -759,7 +759,7 @@ describe('generate-types', () => {
const result = generateTypes.mapPropertyType(prop);
// Should generate nested structure with proper types
expect(result).toContain('systemMessage?:');
expect(result).toContain('string | Expression<string> | PlaceholderValue');
expect(result).toContain('string | Expression<string>');
expect(result).toContain('maxIterations?:');
expect(result).toContain('number | Expression<number>');
expect(result).toContain('returnIntermediateSteps?:');
@ -845,7 +845,7 @@ describe('generate-types', () => {
expect(result).toContain('@builderHint You can add multiple intervals');
});
// PlaceholderValue tests - string type should include PlaceholderValue, other types should not
// PlaceholderValue is no longer emitted by codegen — these tests guard against regressions.
it('should NOT include PlaceholderValue in options type', () => {
const prop: NodeProperty = {
name: 'method',
@ -2586,7 +2586,7 @@ describe('generate-types', () => {
default: null,
};
const result = generateTypes.mapPropertyType(prop);
expect(result).toBe('string | Expression<string> | PlaceholderValue');
expect(result).toBe('string | Expression<string>');
});
it('should handle options with numeric values', () => {

View File

@ -744,10 +744,7 @@ function mapNestedPropertyTypeInner(
switch (prop.type) {
case 'string': {
if (prop.builderHint?.placeholderSupported === false) {
return 'string | Expression<string>';
}
return 'string | Expression<string> | PlaceholderValue';
return 'string | Expression<string>';
}
case 'number':
return 'number | Expression<number>';
@ -882,6 +879,12 @@ function generateNestedPropertyJSDoc(
lines.push(`${indent} * @builderHint ${safeBuilderHint}`);
}
// Placeholder support flag — signals to the builder agent (and the runtime
// guard) that placeholder() is rejected for this parameter.
if (prop.builderHint?.placeholderSupported === false) {
lines.push(`${indent} * @placeholderSupported false`);
}
// Search/load method annotations — signals to the builder agent that
// explore-node-resources can resolve real IDs for this parameter.
if (prop.modes) {
@ -1375,14 +1378,11 @@ function generateCollectionType(
}
/**
* Strip Expression<...> and PlaceholderValue from a type string.
* Strip Expression<...> from a type string.
* Used when noDataExpression is true to produce plain types.
*/
function stripExpressionFromType(typeStr: string): string {
return typeStr
.replace(/\s*\|\s*Expression<[^>]+>/g, '')
.replace(/\s*\|\s*PlaceholderValue/g, '')
.trim();
return typeStr.replace(/\s*\|\s*Expression<[^>]+>/g, '').trim();
}
/**
@ -1436,10 +1436,7 @@ function mapPropertyTypeInner(
switch (prop.type) {
case 'string': {
if (prop.builderHint?.placeholderSupported === false) {
return 'string | Expression<string>';
}
return 'string | Expression<string> | PlaceholderValue';
return 'string | Expression<string>';
}
case 'number':
@ -1908,6 +1905,12 @@ export function generatePropertyJSDoc(
lines.push(` * @builderHint ${safeBuilderHint}`);
}
// Placeholder support flag — signals to the builder agent (and the runtime
// guard) that placeholder() is rejected for this parameter.
if (prop.builderHint?.placeholderSupported === false) {
lines.push(' * @placeholderSupported false');
}
// Search/load method annotations — signals to the builder agent that
// explore-node-resources can resolve real IDs for this parameter.
if (prop.modes) {

View File

@ -41,7 +41,6 @@ export type {
// Split in batches types
SplitInBatchesBuilder,
// Other types
PlaceholderValue,
NewCredentialValue,
AllItemsContext,
EachItemContext,

View File

@ -74,18 +74,6 @@ export interface NewCredentialValue {
readonly id?: string;
}
// =============================================================================
// Placeholder Values
// =============================================================================
/**
* Placeholder for values the user needs to fill in.
*/
export interface PlaceholderValue {
readonly __placeholder: true;
readonly hint: string;
}
// =============================================================================
// Error Handling
// =============================================================================
@ -432,7 +420,10 @@ export interface WorkflowContext {
*/
export interface NodeConfig<TParams = IDataObject> {
parameters?: TParams;
credentials?: Record<string, CredentialReference | NewCredentialValue | PlaceholderValue>;
credentials?: Record<
string,
string | CredentialReference | NewCredentialValue | { value: string }
>;
name?: string;
position?: [number, number];
webhookId?: string;
@ -1169,7 +1160,7 @@ export type StickyFn = (
config?: StickyNoteConfig,
) => NodeInstance<'n8n-nodes-base.stickyNote', 'v1', void>;
export type PlaceholderFn = (hint: string) => PlaceholderValue;
export type PlaceholderFn = (hint: string) => string;
export type NewCredentialFn = (name: string, id?: string) => NewCredentialValue;

View File

@ -8,6 +8,7 @@ import { resolveMainInputCount } from './input-resolver';
import { validateNodeConfig } from './schema-validator';
import { isStickyNoteType, isHttpRequestType } from '../constants/node-types';
import type { WorkflowBuilder, WorkflowJSON } from '../types/base';
import { containsPlaceholderMarker } from '../workflow-builder/string-utils';
export {
setSchemaBaseDirs,
@ -502,6 +503,8 @@ export function validateWorkflow(
validateRequiredInputsConnected(json, options.nodeTypesProvider, errors);
// Validate that emitted connection types are actually exposed by the source node's mode
validateOutputUsage(json, options.nodeTypesProvider, warnings);
// Reject placeholder() in slots that opt out via builderHint.placeholderSupported === false
validatePlaceholderSlots(json, options.nodeTypesProvider, errors);
}
// Merge node input-count consistency
@ -1016,6 +1019,58 @@ function validateOutputUsage(
}
}
/**
* Reject `placeholder()` markers found in parameter slots whose property
* description carries `builderHint.placeholderSupported === false`.
*
* This is the runtime side of the type-level signal that used to live in the
* generated `string | Expression<string>` union (which previously omitted
* `PlaceholderValue`). Now that `placeholder()` returns a plain `string`, the
* type system can no longer block placement; this validator does at runtime.
*
* Uses `containsPlaceholderMarker` (not `isPlaceholderValue`) so that the
* marker is rejected anywhere in the value including `expr(placeholder())`,
* which produces `=<__PLACEHOLDER_VALUE__…__>`, and placeholders embedded
* inside `={{ … }}` expressions.
*
* Walks top-level properties only the known declarations
* (webhook `path`, langchain agent `text`) are top-level fields. Nested
* collection / fixedCollection support can be added later if a node opts out
* of placeholders for a nested field.
*/
function validatePlaceholderSlots(
json: WorkflowJSON,
nodeTypesProvider: INodeTypes,
errors: ValidationError[],
): void {
for (const node of json.nodes) {
if (!node.name || !node.parameters) continue;
const version =
typeof node.typeVersion === 'string' ? parseFloat(node.typeVersion) : (node.typeVersion ?? 1);
const nodeType = nodeTypesProvider.getByNameAndVersion(node.type, version);
const properties = nodeType?.description?.properties;
if (!properties) continue;
const params = node.parameters as Record<string, unknown>;
for (const prop of properties) {
if (prop.builderHint?.placeholderSupported !== false) continue;
const value = params[prop.name];
if (!containsPlaceholderMarker(value)) continue;
errors.push(
new ValidationError(
'INVALID_PARAMETER',
`Node "${node.name}": placeholder() is not supported for parameter '${prop.name}'. Use a literal value or expr() instead.`,
node.name,
prop.name,
),
);
}
}
}
/**
* Check if connections use valid input indices for their target nodes.
* Reports warnings for connections to input indices that don't exist.

View File

@ -2987,4 +2987,119 @@ describe('Validation', () => {
expect(warnings).toHaveLength(0);
});
});
describe('validatePlaceholderSlots (builderHint.placeholderSupported=false)', () => {
const mockNodeTypesProviderWithPlaceholderOptOut = {
getByNameAndVersion: (_type: string, _version?: number) => ({
description: {
inputs: ['main'],
properties: [
{
name: 'path',
displayName: 'Path',
type: 'string',
default: '',
builderHint: { placeholderSupported: false },
},
{
name: 'method',
displayName: 'Method',
type: 'string',
default: 'GET',
},
],
},
}),
getByName: (type: string) =>
mockNodeTypesProviderWithPlaceholderOptOut.getByNameAndVersion(type),
getKnownTypes: () => ({}),
};
const makeWorkflow = (paramValue: string) => ({
id: 'test',
name: 'Test',
nodes: [
{
id: 'webhook-1',
name: 'Webhook',
type: 'n8n-nodes-base.webhook',
typeVersion: 1,
position: [0, 0] as [number, number],
parameters: { path: paramValue },
},
],
connections: {},
});
it('rejects bare placeholder() marker', () => {
const result = validateWorkflow(makeWorkflow('<__PLACEHOLDER_VALUE__my path__>'), {
nodeTypesProvider: mockNodeTypesProviderWithPlaceholderOptOut as never,
});
const errors = result.errors.filter(
(e) => e.code === 'INVALID_PARAMETER' && e.message.includes('placeholder()'),
);
expect(errors).toHaveLength(1);
expect(errors[0].nodeName).toBe('Webhook');
expect(errors[0].parameterName).toBe('path');
});
it('rejects placeholder() wrapped in expr() — leading "=" prefix', () => {
const result = validateWorkflow(makeWorkflow('=<__PLACEHOLDER_VALUE__my path__>'), {
nodeTypesProvider: mockNodeTypesProviderWithPlaceholderOptOut as never,
});
const errors = result.errors.filter(
(e) => e.code === 'INVALID_PARAMETER' && e.message.includes('placeholder()'),
);
expect(errors).toHaveLength(1);
expect(errors[0].nodeName).toBe('Webhook');
expect(errors[0].parameterName).toBe('path');
});
it('rejects placeholder() embedded inside a larger expression', () => {
const result = validateWorkflow(
makeWorkflow('={{ "prefix-" + "<__PLACEHOLDER_VALUE__my path__>" }}'),
{ nodeTypesProvider: mockNodeTypesProviderWithPlaceholderOptOut as never },
);
const errors = result.errors.filter(
(e) => e.code === 'INVALID_PARAMETER' && e.message.includes('placeholder()'),
);
expect(errors).toHaveLength(1);
});
it('does not flag literal values', () => {
const result = validateWorkflow(makeWorkflow('webhook-path'), {
nodeTypesProvider: mockNodeTypesProviderWithPlaceholderOptOut as never,
});
const errors = result.errors.filter(
(e) => e.code === 'INVALID_PARAMETER' && e.message.includes('placeholder()'),
);
expect(errors).toHaveLength(0);
});
it('does not flag placeholder() in slots without the opt-out hint', () => {
const provider = {
getByNameAndVersion: () => ({
description: {
inputs: ['main'],
properties: [{ name: 'path', displayName: 'Path', type: 'string', default: '' }],
},
}),
getByName: () => provider.getByNameAndVersion(),
getKnownTypes: () => ({}),
};
const result = validateWorkflow(makeWorkflow('=<__PLACEHOLDER_VALUE__my path__>'), {
nodeTypesProvider: provider as never,
});
const errors = result.errors.filter(
(e) => e.code === 'INVALID_PARAMETER' && e.message.includes('placeholder()'),
);
expect(errors).toHaveLength(0);
});
});
});

View File

@ -354,16 +354,15 @@ describe('Node Builder', () => {
});
describe('placeholder()', () => {
it('should create a placeholder value with hint', () => {
it('returns the placeholder marker string', () => {
const p = placeholder('Enter Channel');
expect(p.__placeholder).toBe(true);
expect(p.hint).toBe('Enter Channel');
expect(typeof p).toBe('string');
expect(p).toBe('<__PLACEHOLDER_VALUE__Enter Channel__>');
});
it('should serialize to placeholder format', () => {
it('JSON-serializes to the placeholder marker', () => {
const p = placeholder('API Key');
// eslint-disable-next-line @typescript-eslint/no-base-to-string -- Testing custom toString behavior
expect(String(p)).toBe('<__PLACEHOLDER_VALUE__API Key__>');
expect(JSON.stringify({ key: p })).toBe('{"key":"<__PLACEHOLDER_VALUE__API Key__>"}');
});
});
@ -461,8 +460,8 @@ describe('Node Builder', () => {
expect((stored as { __newCredential?: boolean }).__newCredential).toBe(true);
expect((stored as { name?: string }).name).toBe('Slack Bot');
expect((stored as { id?: string }).id).toBeUndefined();
// The original __placeholder marker is gone — credentials maps never carry it.
expect((stored as { __placeholder?: boolean }).__placeholder).toBeUndefined();
// The placeholder marker string is gone — credentials maps never carry it.
expect(typeof stored).not.toBe('string');
});
it('serializes a placeholder() credential to undefined (omitted from JSON)', () => {
@ -521,6 +520,63 @@ describe('Node Builder', () => {
expect((stored as { __newCredential?: boolean }).__newCredential).toBe(true);
expect((stored as { name?: string }).name).toBe('Slack Bot');
});
it('normalizes { value: placeholder() } shape to newCredential', () => {
const n = node({
type: 'n8n-nodes-base.slack',
version: 2.2,
config: { credentials: { slackApi: { value: placeholder('Slack token') } } },
});
const stored = n.config.credentials?.slackApi as { __newCredential?: boolean; name?: string };
expect(stored.__newCredential).toBe(true);
expect(stored.name).toBe('Slack token');
});
it('normalizes { value: literal } shape to newCredential', () => {
const n = node({
type: 'n8n-nodes-base.slack',
version: 2.2,
config: { credentials: { slackApi: { value: 'My Slack Token' } } },
});
const stored = n.config.credentials?.slackApi as { __newCredential?: boolean; name?: string };
expect(stored.__newCredential).toBe(true);
expect(stored.name).toBe('My Slack Token');
});
it('normalizes { id: placeholder, name: literal } to newCredential keyed by name', () => {
const n = node({
type: 'n8n-nodes-base.slack',
version: 2.2,
config: { credentials: { slackApi: { id: placeholder('id hint'), name: 'Slack Bot' } } },
});
const stored = n.config.credentials?.slackApi as { __newCredential?: boolean; name?: string };
expect(stored.__newCredential).toBe(true);
expect(stored.name).toBe('Slack Bot');
});
it('normalizes { id: placeholder, name: placeholder } to newCredential keyed by name hint', () => {
const n = node({
type: 'n8n-nodes-base.slack',
version: 2.2,
config: {
credentials: {
slackApi: { id: placeholder('id hint'), name: placeholder('Slack Bot') },
},
},
});
const stored = n.config.credentials?.slackApi as { __newCredential?: boolean; name?: string };
expect(stored.__newCredential).toBe(true);
expect(stored.name).toBe('Slack Bot');
});
it('passes a raw {id, name} CredentialReference through unchanged', () => {
const n = node({
type: 'n8n-nodes-base.slack',
version: 2.2,
config: { credentials: { slackApi: { id: 'cred-1', name: 'Slack Bot' } } },
});
expect(n.config.credentials?.slackApi).toEqual({ id: 'cred-1', name: 'Slack Bot' });
});
});
describe('then() with multiple targets (fan-out)', () => {

View File

@ -9,7 +9,6 @@ import {
type NodeInput,
type TriggerInput,
type StickyNoteConfig,
type PlaceholderValue,
type NewCredentialValue,
type CredentialReference,
type DeclaredConnection,
@ -21,6 +20,7 @@ import {
type IfElseTarget,
type SwitchCaseTarget,
} from '../../types/base';
import { extractHint, isPlaceholderValue } from '../string-utils';
import {
isSwitchCaseComposite,
isIfElseComposite,
@ -100,32 +100,77 @@ function generateNodeName(type: string): string {
}
/**
* Collapse `placeholder('hint')` markers inside a credentials map into
* `newCredential('hint')`. The two have identical intent in this slot
* "a credential is required, no real one is bound yet" so we normalize at
* config ingest. Downstream code (resolveCredentials, hasNewCredential, the
* `__newCredential` toJSON path) only ever sees `__newCredential` markers in
* credential slots, never `__placeholder` ones.
* Normalize the various credential-slot shapes the agent (or hand-written code)
* may emit into the canonical `CredentialReference | NewCredentialValue` form
* downstream code expects.
*
* Accepted shapes (all converge to `NewCredentialValue` or `CredentialReference`):
* - bare placeholder marker string `newCredential(extractedHint)`
* - `{ value: <string|placeholder> }` `newCredential(<hint or literal>)`
* - `{ id: <placeholder>, name: <string|placeholder> }` `newCredential(<name or id-hint>)`
* - `{ id: <string>, name: <string> }` unchanged `CredentialReference`
* - `NewCredentialValue` instance unchanged
*
* Returns a new config object when any normalization happens; otherwise a
* shallow copy (matching the previous `{ ...config }` semantics).
*/
export function normalizeNodeConfig(config: NodeConfig): NodeConfig {
const creds = config?.credentials;
const creds = config?.credentials as Record<string, unknown> | undefined;
if (!creds) return { ...config };
let normalizedCreds:
| Record<string, CredentialReference | NewCredentialValue | PlaceholderValue>
| Record<string, CredentialReference | NewCredentialValue | string>
| undefined;
const setNormalized = (key: string, next: CredentialReference | NewCredentialValue | string) => {
normalizedCreds ??= { ...creds } as Record<
string,
CredentialReference | NewCredentialValue | string
>;
normalizedCreds[key] = next;
};
for (const [key, value] of Object.entries(creds)) {
if (value && typeof value === 'object' && '__placeholder' in value) {
normalizedCreds ??= { ...creds };
normalizedCreds[key] = new NewCredentialImpl(value.hint);
// 1. Bare placeholder marker string → newCredential(extractedHint)
if (typeof value === 'string' && isPlaceholderValue(value)) {
setNormalized(key, new NewCredentialImpl(extractHint(value)));
continue;
}
if (!value || typeof value !== 'object') {
continue;
}
const obj = value as Record<string, unknown>;
// Already a NewCredentialValue — leave alone
if ('__newCredential' in obj) continue;
// 2. { value: ... } → newCredential
if ('value' in obj && !('id' in obj)) {
const v = String(obj.value);
const name = isPlaceholderValue(v) ? extractHint(v) : v;
setNormalized(key, new NewCredentialImpl(name));
continue;
}
// 3. { id, name } where id is a placeholder marker → newCredential(name)
if ('id' in obj) {
const idStr = typeof obj.id === 'string' ? obj.id : '';
if (isPlaceholderValue(idStr)) {
const rawName = obj.name;
const name =
typeof rawName === 'string' && rawName.length > 0 && !isPlaceholderValue(rawName)
? rawName
: typeof rawName === 'string' && isPlaceholderValue(rawName)
? extractHint(rawName)
: extractHint(idStr);
setNormalized(key, new NewCredentialImpl(name));
continue;
}
}
// 4. else: leave as CredentialReference / plain string
}
if (!normalizedCreds) return { ...config };
return { ...config, credentials: normalizedCreds };
return { ...config, credentials: normalizedCreds } as NodeConfig;
}
/**
@ -1159,38 +1204,23 @@ export function sticky(
}
/**
* Placeholder implementation
*/
class PlaceholderImpl implements PlaceholderValue {
readonly __placeholder = true as const;
readonly hint: string;
constructor(hint: string) {
this.hint = hint;
}
toString(): string {
return `<__PLACEHOLDER_VALUE__${this.hint}__>`;
}
toJSON(): string {
return this.toString();
}
}
/**
* Create a placeholder value for template parameters
* Create a placeholder value for template parameters.
*
* Placeholders are used to mark values that need to be filled in
* when a workflow template is instantiated.
* Returns the marker string `<__PLACEHOLDER_VALUE__<hint>__>`, which is
* structurally a `string` so it flows through every string-typed slot in
* generated node types without TypeScript complaints. The workflow compiler
* recognises the marker via {@link isPlaceholderValue} and treats the slot as
* "to be filled in by the user before execution".
*
* Inside a node's `credentials` slot, `placeholder(hint)` is normalized to
* `newCredential(hint)` at config ingest the two have identical intent
* there ("a credential is required, no real one bound yet"). Outside the
* credentials slot the original placeholder semantics are unchanged.
* Inside a node's `credentials` slot, the marker is normalised to
* `newCredential(hint)` at config ingest see {@link normalizeNodeConfig}.
*
* Note: a small number of parameters carry `placeholderSupported: false` in
* their description (e.g. webhook `path`) and the workflow compiler will throw
* a clear error if a placeholder lands in such a slot.
*
* @param hint - Description shown to users (e.g., 'Enter Channel')
* @returns A placeholder value that serializes to the placeholder format
* @returns The placeholder marker string.
*
* @example
* ```typescript
@ -1200,8 +1230,8 @@ class PlaceholderImpl implements PlaceholderValue {
* // Serializes channel as: '<__PLACEHOLDER_VALUE__Enter Channel__>'
* ```
*/
export function placeholder(hint: string): PlaceholderValue {
return new PlaceholderImpl(hint);
export function placeholder(hint: string): string {
return `<__PLACEHOLDER_VALUE__${hint}__>`;
}
/**

View File

@ -161,12 +161,12 @@ describe('isDataTableWithoutTable', () => {
expect(isDataTableWithoutTable(node)).toBe(true);
});
it('returns true for dataTable node with placeholder value', () => {
it('returns true for dataTable node with placeholder marker value', () => {
const node = createNode({
type: 'n8n-nodes-base.dataTable',
config: {
parameters: {
dataTableId: { value: { __placeholder: true } },
dataTableId: { value: '<__PLACEHOLDER_VALUE__Select table__>' },
},
},
});

View File

@ -4,6 +4,7 @@
* Functions for determining which nodes should have pin data generated.
*/
import { isPlaceholderValue } from './string-utils';
import { isHttpRequestType, isWebhookType, isDataTableType } from '../constants/node-types';
import type { NodeInstance } from '../types/base';
@ -12,11 +13,10 @@ import type { NodeInstance } from '../types/base';
* Nodes with new credentials need pin data to avoid execution errors.
*
* Note: by the time a NodeInstance reaches this code, credentials slots only
* ever contain `CredentialReference` or `__newCredential` markers never
* `__placeholder` markers. `placeholder()` values supplied for credentials
* are normalized to `__newCredential` at config ingest in
* `node-builder.ts#normalizeNodeConfig`, so we don't need a second check
* here.
* ever contain `CredentialReference` or `__newCredential` markers. Bare
* placeholder marker strings and the `{ value }` convenience shape supplied
* for credentials are normalized to `__newCredential` at config ingest in
* `node-builder.ts#normalizeNodeConfig`, so we don't need a second check here.
*/
export function hasNewCredential(node: NodeInstance<string, string, unknown>): boolean {
// Check main node credentials
@ -75,12 +75,8 @@ export function isDataTableWithoutTable(node: NodeInstance<string, string, unkno
return true;
}
// Check if value is a placeholder (user needs to fill in)
if (
typeof dataTableId.value === 'object' &&
dataTableId.value !== null &&
'__placeholder' in dataTableId.value
) {
// Check if value is a placeholder marker (user needs to fill in)
if (isPlaceholderValue(dataTableId.value)) {
return true;
}

View File

@ -108,14 +108,22 @@ function serializeNode(
// post-redaction string `"[REDACTED]"`). Pass through unchanged.
n8nNode.credentials = deepCopy(config.credentials);
} else {
// `NodeConfig.credentials` is typed wide (also accepts PlaceholderValue)
// at the public API. By this point `normalizeNodeConfig` has rewritten any
// placeholder() markers to newCredential() markers, so no __placeholder
// values remain at runtime. Narrow the value type for the serializer.
// `NodeConfig.credentials` is typed wide (string | { value } | etc.) at the
// public API. By this point `normalizeNodeConfig` has rewritten the loose
// shapes to `CredentialReference | NewCredentialValue`. Defensively skip
// any leftover placeholder marker strings or `{ value }` objects (they are
// placeholders the user must still fill in and have no `id`/`name` to
// serialize). Plain strings (e.g. legacy 'YOUR_CREDENTIALS' style refs)
// pass through unchanged for backwards compatibility.
const resolvable: NonNullable<NodeJSON['credentials']> = {};
for (const [key, value] of Object.entries(config.credentials)) {
if (value && typeof value === 'object' && '__placeholder' in value) continue;
resolvable[key] = value;
if (typeof value === 'string') {
if (value.startsWith('<__PLACEHOLDER_VALUE__') && value.endsWith('__>')) continue;
resolvable[key] = value as unknown as { id?: string; name: string };
continue;
}
if (value && typeof value === 'object' && 'value' in value && !('id' in value)) continue;
resolvable[key] = value as { id?: string; name: string };
}
n8nNode.credentials = deepCopy(resolvable);
}

View File

@ -5,6 +5,7 @@ import {
filterMethodsFromPath,
parseVersion,
isPlaceholderValue,
containsPlaceholderMarker,
isResourceLocatorLike,
normalizeResourceLocators,
escapeNewlinesInStringLiterals,
@ -106,6 +107,38 @@ describe('workflow-builder/string-utils', () => {
});
});
describe('containsPlaceholderMarker', () => {
it('returns true for bare placeholder string', () => {
expect(containsPlaceholderMarker('<__PLACEHOLDER_VALUE__test__>')).toBe(true);
});
it('returns true for expr()-wrapped placeholder (leading "=")', () => {
expect(containsPlaceholderMarker('=<__PLACEHOLDER_VALUE__test__>')).toBe(true);
});
it('returns true for placeholder embedded in an expression block', () => {
expect(containsPlaceholderMarker('={{ "prefix-" + "<__PLACEHOLDER_VALUE__name__>" }}')).toBe(
true,
);
});
it('returns false for literal strings without the marker', () => {
expect(containsPlaceholderMarker('regular string')).toBe(false);
expect(containsPlaceholderMarker('=expression')).toBe(false);
});
it('returns false for incomplete marker (missing closing)', () => {
expect(containsPlaceholderMarker('<__PLACEHOLDER_VALUE__test')).toBe(false);
});
it('returns false for non-string values', () => {
expect(containsPlaceholderMarker(123)).toBe(false);
expect(containsPlaceholderMarker(null)).toBe(false);
expect(containsPlaceholderMarker(undefined)).toBe(false);
expect(containsPlaceholderMarker({})).toBe(false);
});
});
describe('isResourceLocatorLike', () => {
it('returns true for object with mode and value', () => {
expect(isResourceLocatorLike({ mode: 'list', value: 'test' })).toBe(true);

View File

@ -68,6 +68,31 @@ export function isPlaceholderValue(value: unknown): boolean {
return value.startsWith('<__PLACEHOLDER_VALUE__') && value.endsWith('__>');
}
/**
* Check if a value contains a placeholder marker anywhere within it.
*
* Unlike {@link isPlaceholderValue}, which requires the marker to span the
* entire string, this catches placeholders embedded in larger strings most
* notably `=<__PLACEHOLDER_VALUE__…__>` produced by wrapping `placeholder()`
* inside `expr()`, or placeholders concatenated inside `={{ … }}` blocks.
*/
export function containsPlaceholderMarker(value: unknown): boolean {
if (typeof value !== 'string') return false;
return /<__PLACEHOLDER_VALUE__[\s\S]*?__>/.test(value);
}
/**
* Extract the original hint from a placeholder marker string.
* Returns the input unchanged if it does not match the marker format.
*
* @example
* extractHint('<__PLACEHOLDER_VALUE__OpenAI key__>') // → 'OpenAI key'
*/
export function extractHint(value: string): string {
if (!isPlaceholderValue(value)) return value;
return value.slice('<__PLACEHOLDER_VALUE__'.length, -'__>'.length);
}
/**
* Check if an object looks like a resource locator value.
* Resource locators have a 'mode' property (typically 'list', 'id', 'url', or 'name')

View File

@ -194,12 +194,9 @@ describe('workflow-builder/validation-helpers', () => {
expect(issues[0].path).toBe('items[1]');
});
it('skips PlaceholderValue objects', () => {
it('skips placeholder marker strings', () => {
const issues = findMissingExpressionPrefixes({
field: {
__placeholder: true,
hint: '{{ $json.field }}',
},
field: '<__PLACEHOLDER_VALUE__{{ $json.field }}__>',
});
expect(issues).toHaveLength(0);
});

View File

@ -3,6 +3,7 @@
* Pure functions extracted from WorkflowBuilderImpl
*/
import { isPlaceholderValue } from './string-utils';
import { isTriggerNodeType } from '../utils/trigger-detection';
/**
@ -124,6 +125,8 @@ export function findMissingExpressionPrefixes(
const issues: Array<{ path: string; value: string }> = [];
if (typeof value === 'string') {
// Skip placeholder markers — their embedded hint is documentation, not an expression.
if (isPlaceholderValue(value)) return issues;
// If string starts with '=', it's already an expression - {{ }} is valid template syntax inside
// Otherwise check if it contains {{ $ pattern (n8n variable reference without = prefix)
if (!value.startsWith('=') && value.includes('{{ $')) {
@ -134,10 +137,6 @@ export function findMissingExpressionPrefixes(
issues.push(...findMissingExpressionPrefixes(item, `${path}[${index}]`));
});
} else if (value && typeof value === 'object') {
// Skip PlaceholderValue objects - their hint property is documentation, not actual expressions
if ('__placeholder' in value && (value as { __placeholder: boolean }).__placeholder) {
return issues;
}
for (const [key, val] of Object.entries(value)) {
const newPath = path ? `${path}.${key}` : key;
issues.push(...findMissingExpressionPrefixes(val, newPath));
@ -181,6 +180,7 @@ export function findInvalidDateMethods(
const issues: Array<{ path: string; value: string }> = [];
if (typeof value === 'string') {
if (isPlaceholderValue(value)) return issues;
if (hasLuxonToISOStringMisuse(value)) {
issues.push({ path, value });
}
@ -189,10 +189,6 @@ export function findInvalidDateMethods(
issues.push(...findInvalidDateMethods(item, `${path}[${index}]`));
});
} else if (value && typeof value === 'object') {
// Skip PlaceholderValue objects
if ('__placeholder' in value && (value as { __placeholder: boolean }).__placeholder) {
return issues;
}
for (const [key, val] of Object.entries(value)) {
const newPath = path ? `${path}.${key}` : key;
issues.push(...findInvalidDateMethods(val, newPath));