From d6965475daef3bf9d3c852e8704448644ca8715e Mon Sep 17 00:00:00 2001 From: sezallagwal Date: Sun, 21 Jun 2026 20:03:20 +0530 Subject: [PATCH 1/2] Add workflow composition normalization --- src/composer/normalization.ts | 618 ++++++++++++++++++++++++++++++++++ 1 file changed, 618 insertions(+) create mode 100644 src/composer/normalization.ts diff --git a/src/composer/normalization.ts b/src/composer/normalization.ts new file mode 100644 index 0000000..70482bf --- /dev/null +++ b/src/composer/normalization.ts @@ -0,0 +1,618 @@ +import type { JSONSchema7 } from "json-schema"; +import { INPUT_SCHEMA_BODY_KEY } from "../parser/types.js"; +import type { + ApiCallStep, + ConditionalStep, + ElicitationStep, + SamplingStep, + TransformStep, +} from "../workflow/types.js"; +import { ComposerError, type ComposerWarning, type ComposeStepInput } from "./types.js"; + +export function normalizeStepFields(steps: ComposeStepInput[]): ComposerWarning[] { + const warnings: ComposerWarning[] = []; + for (const step of steps) { + const cfg = step.config as unknown as Record; + if (cfg.type === "api_call") { + if (cfg.as && !cfg.forEach) { + delete cfg.as; + warnings.push({ + stepId: step.id, + code: "FIELD_STRIPPED", + message: `Stripped "as" from step "${step.id}" — "as" is only used with "forEach" for iteration. Step results are accessed via steps.${step.id}`, + }); + } + if (cfg.forEach && !cfg.as) { + cfg.as = `${step.id}_item`; + warnings.push({ + stepId: step.id, + code: "FIELD_AUTO_SET", + message: `Auto-set as="${cfg.as}" for step "${step.id}" — "as" names the loop variable in forEach iteration`, + }); + } + } + } + return warnings; +} + + +export function normalizeEventParamShorthand( + steps: ComposeStepInput[], + params: JSONSchema7, +): ComposerWarning[] { + const warnings: ComposerWarning[] = []; + const paramProps = params.properties ? Object.keys(params.properties) : []; + if (paramProps.length === 0) return warnings; + + // Collect forEach iteration variable names so we don't rewrite them as params + const forEachAsVars = new Set(); + for (const step of steps) { + if (step.config.type === "api_call" && (step.config as ApiCallStep).as) { + forEachAsVars.add((step.config as ApiCallStep).as!); + } + } + + const templateRewriters = paramProps + .filter((name) => !forEachAsVars.has(name)) + .map((name) => ({ + name, + re: new RegExp(`\\{\\{(?!params\\.)${name}\\.`, "g"), + replacement: `{{params.${name}.`, + })); + + const jsRewriters = paramProps + .filter((name) => !forEachAsVars.has(name)) + .map((name) => ({ + name, + re: new RegExp(`(? + rewriteValue(stepId, item, `${fieldName}[${i}]`), + ); + if (typeof value === "object" && value !== null) { + const result: Record = {}; + for (const [k, v] of Object.entries(value)) { + result[k] = rewriteValue(stepId, v, `${fieldName}.${k}`); + } + return result; + } + return value; + } + + for (const step of steps) { + const cfg = step.config; + switch (cfg.type) { + case "api_call": { + const apiCfg = cfg as ApiCallStep; + if (apiCfg.inputMapping) { + apiCfg.inputMapping = rewriteValue( + step.id, + apiCfg.inputMapping, + "inputMapping", + ) as Record; + } + if (apiCfg.forEach) { + apiCfg.forEach = rewriteTemplate(step.id, apiCfg.forEach, "forEach"); + } + break; + } + case "sampling": { + const sCfg = cfg as SamplingStep; + sCfg.prompt = rewriteTemplate(step.id, sCfg.prompt, "prompt"); + if (sCfg.systemPrompt) + sCfg.systemPrompt = rewriteTemplate( + step.id, + sCfg.systemPrompt, + "systemPrompt", + ); + if (sCfg.content) { + for (const item of sCfg.content) { + if (item.type === "text") + item.text = rewriteTemplate(step.id, item.text, "content.text"); + } + } + break; + } + case "elicitation": { + const eCfg = cfg as ElicitationStep; + eCfg.message = rewriteTemplate(step.id, eCfg.message, "message"); + break; + } + case "conditional": { + const cCfg = cfg as ConditionalStep; + cCfg.condition = rewriteJs(step.id, cCfg.condition, "condition"); + break; + } + case "transform": { + const tCfg = cfg as TransformStep; + tCfg.expression = rewriteJs(step.id, tCfg.expression, "expression"); + break; + } + } + } + + return warnings; +} + +/** Escape for double-quoted JS string literals. */ +function escapeStringLiteral(s: string): string { + return s + .replace(/\\/g, "\\\\") + .replace(/"/g, '\\"') + .replace(/\n/g, "\\n") + .replace(/\r/g, "\\r") + .replace(/\t/g, "\\t"); +} + +/** Convert {{#each}}/{{#if}} Handlebars blocks to JS expressions; throws on unsupported/nested blocks. */ +function convertHandlebarsBlocks( + input: string, + stepId: string, + fieldName: string, + warnings: ComposerWarning[], +): string { + let result = input; + + // Detect unsupported block helpers ({{#unless}}, {{#with}}, etc.) + const unsupportedBlock = result.match(/\{\{#(?!each\b|if\b)(\w+)/); + if (unsupportedBlock) { + throw new ComposerError( + `Step "${stepId}" field "${fieldName}" uses unsupported Handlebars helper "{{#${unsupportedBlock[1]}}}". ` + + `The template engine uses {{jsExpression}} syntax. ` + + `Use JavaScript expressions instead (e.g. array.map(), ternary operators).`, + ); + } + + // Convert {{#each collection}}...body...{{/each}} + const eachRe = /\{\{#each\s+([^}]+)\}\}([\s\S]*?)\{\{\/each\}\}/g; + result = result.replace(eachRe, (_, collection: string, body: string) => { + const col = collection.trim(); + + // Check for nested blocks — bail with clear error + if (/\{\{#(each|if)\b/.test(body)) { + throw new ComposerError( + `Step "${stepId}" field "${fieldName}" uses nested Handlebars blocks which cannot be auto-converted. ` + + `Use JavaScript expressions instead. Example: ` + + `{{${col}.map(item => item.name + ": " + item.value).join("\\n")}}`, + ); + } + + // Split body into static parts and dynamic {{this.X}} / {{this}} references + // Build: collection.map(item => "static" + (item.field ?? "") + "static").join("") + const parts: string[] = []; + let lastIndex = 0; + const refRe = /\{\{this(?:\.(\w+(?:\.\w+)*))?\}\}/g; + let match; + while ((match = refRe.exec(body)) !== null) { + // Static part before this reference + if (match.index > lastIndex) { + parts.push( + `"${escapeStringLiteral(body.slice(lastIndex, match.index))}"`, + ); + } + // Dynamic part + const fieldPath = match[1]; + if (fieldPath) { + parts.push(`(item.${fieldPath} ?? "")`); + } else { + parts.push(`(item ?? "")`); + } + lastIndex = match.index + match[0].length; + } + // Trailing static part + if (lastIndex < body.length) { + parts.push(`"${escapeStringLiteral(body.slice(lastIndex))}"`); + } + + const mapBody = parts.length > 0 ? parts.join(" + ") : '""'; + const expr = `{{${col}.map(item => ${mapBody}).join("")}}`; + + // Validate the generated expression compiles + try { + const innerExpr = expr.slice(2, -2); // strip {{ }} + new Function("steps", "params", `"use strict"; return (${innerExpr});`); + } catch { + throw new ComposerError( + `Step "${stepId}" field "${fieldName}": auto-converted Handlebars {{#each}} failed to compile. ` + + `Original: "${_.trim()}". Converted: "${expr}". ` + + `Use JavaScript expressions directly instead.`, + ); + } + + warnings.push({ + stepId, + code: "FIELD_STRIPPED", + message: `Auto-converted Handlebars {{#each}} to JS in ${fieldName}`, + }); + return expr; + }); + + // Convert {{#if cond}}...then...{{else}}...else...{{/if}} + // and {{#if cond}}...then...{{/if}} + const ifElseRe = + /\{\{#if\s+([^}]+)\}\}([\s\S]*?)\{\{else\}\}([\s\S]*?)\{\{\/if\}\}/g; + result = result.replace( + ifElseRe, + (_, condition: string, thenBody: string, elseBody: string) => { + const cond = condition.trim(); + if ( + /\{\{#(each|if)\b/.test(thenBody) || + /\{\{#(each|if)\b/.test(elseBody) + ) { + throw new ComposerError( + `Step "${stepId}" field "${fieldName}" uses nested Handlebars blocks inside {{#if}} which cannot be auto-converted.`, + ); + } + const thenStr = escapeStringLiteral(thenBody); + const elseStr = escapeStringLiteral(elseBody); + warnings.push({ + stepId, + code: "FIELD_STRIPPED", + message: `Auto-converted Handlebars {{#if}}...{{else}} to JS ternary in ${fieldName}`, + }); + return `{{${cond} ? "${thenStr}" : "${elseStr}"}}`; + }, + ); + + const ifOnlyRe = /\{\{#if\s+([^}]+)\}\}([\s\S]*?)\{\{\/if\}\}/g; + result = result.replace( + ifOnlyRe, + (_, condition: string, thenBody: string) => { + const cond = condition.trim(); + if (/\{\{#(each|if)\b/.test(thenBody)) { + throw new ComposerError( + `Step "${stepId}" field "${fieldName}" uses nested Handlebars blocks inside {{#if}} which cannot be auto-converted.`, + ); + } + const thenStr = escapeStringLiteral(thenBody); + warnings.push({ + stepId, + code: "FIELD_STRIPPED", + message: `Auto-converted Handlebars {{#if}} to JS ternary in ${fieldName}`, + }); + return `{{${cond} ? "${thenStr}" : ""}}`; + }, + ); + + return result; +} + +export function normalizeTemplateFields(steps: ComposeStepInput[]): ComposerWarning[] { + const warnings: ComposerWarning[] = []; + + const asVars = new Set(); + for (const step of steps) { + if (step.config.type === "api_call" && (step.config as ApiCallStep).as) { + asVars.add((step.config as ApiCallStep).as!); + } + } + + function normalizeString( + stepId: string, + value: string, + fieldName: string, + ): string { + // Collapse \n / \\n / \\\n → real newline (LLMs often multi-escape in JSON tool args) + let result = value.replace(/\\+n/g, "\n").replace(/\\+t/g, "\t"); + + // First: convert any Handlebars block syntax to JS expressions + result = /\{\{#/.test(result) + ? convertHandlebarsBlocks(result, stepId, fieldName, warnings) + : result; + + if (/^(steps|params)\.\w+(\.\w+)*$/.test(result)) { + const wrapped = `{{${result}}}`; + warnings.push({ + stepId, + code: "TEMPLATE_AUTO_WRAPPED", + message: `Auto-wrapped bare reference in ${fieldName}: "${value}" → "${wrapped}"`, + }); + result = wrapped; + } + + for (const asVar of asVars) { + const asRefRe = new RegExp( + `\\{\\{${asVar}\\.(\\w+(?:\\.\\w+)*)\\}\\}`, + "g", + ); + const rewritten = result.replace(asRefRe, `{{steps.${asVar}.$1}}`); + if (rewritten !== result) { + warnings.push({ + stepId, + code: "AS_VAR_REWRITTEN", + message: `Rewritten as-variable reference in ${fieldName}: "${result}" → "${rewritten}"`, + }); + result = rewritten; + } + } + + // Auto-strip legacy .result. from step references (Gemini training data may still emit it) + const stripped = result.replace(/\{\{(steps\.\w+)\.result\./g, "{{$1."); + if (stripped !== result) { + warnings.push({ + stepId, + code: "FIELD_STRIPPED", + message: `Auto-stripped legacy .result from step reference in ${fieldName}: "${result}" → "${stripped}"`, + }); + result = stripped; + } + + return result; + } + + function normalizeValue( + stepId: string, + value: unknown, + fieldName: string, + ): unknown { + if (typeof value === "string") { + // Detect stringified JSON objects/arrays and parse them back to native types + if (/^\s*[\[{]/.test(value)) { + try { + const parsed = JSON.parse(value); + if (typeof parsed === "object" && parsed !== null) { + warnings.push({ + stepId, + code: "STRINGIFIED_JSON_PARSED", + message: `Auto-parsed stringified JSON in ${fieldName}: "${value.length > 60 ? value.slice(0, 60) + "..." : value}"`, + }); + return normalizeValue(stepId, parsed, fieldName); + } + } catch { } + } + return normalizeString(stepId, value, fieldName); + } + if (Array.isArray(value)) { + return value.map((item, i) => + normalizeValue(stepId, item, `${fieldName}[${i}]`), + ); + } + if (typeof value === "object" && value !== null) { + const result: Record = {}; + for (const [k, v] of Object.entries(value)) { + result[k] = normalizeValue(stepId, v, `${fieldName}.${k}`); + } + return result; + } + return value; + } + + for (const step of steps) { + const cfg = step.config; + switch (cfg.type) { + case "api_call": { + const apiCfg = cfg as ApiCallStep; + + if (apiCfg.inputMapping) { + const keys = Object.keys(apiCfg.inputMapping); + if ( + keys.length === 1 && + (keys[0] === INPUT_SCHEMA_BODY_KEY || keys[0] === "body") + ) { + const inner = apiCfg.inputMapping[keys[0]]; + if ( + typeof inner === "object" && + inner !== null && + !Array.isArray(inner) + ) { + apiCfg.inputMapping = inner as Record; + warnings.push({ + stepId: step.id, + code: "REQUEST_BODY_UNWRAPPED", + message: `Auto-unwrapped "${keys[0]}" wrapper in inputMapping for step "${step.id}".`, + }); + } + } + } + + if (apiCfg.inputMapping) { + apiCfg.inputMapping = normalizeValue( + step.id, + apiCfg.inputMapping, + "inputMapping", + ) as Record; + } + if (apiCfg.forEach) { + apiCfg.forEach = normalizeString(step.id, apiCfg.forEach, "forEach"); + } + break; + } + case "sampling": { + const sCfg = cfg as SamplingStep; + sCfg.prompt = normalizeString(step.id, sCfg.prompt, "prompt"); + if (sCfg.systemPrompt) { + sCfg.systemPrompt = normalizeString( + step.id, + sCfg.systemPrompt, + "systemPrompt", + ); + } + if (sCfg.content) { + for (const item of sCfg.content) { + if (item.type === "text") { + item.text = normalizeString(step.id, item.text, "content.text"); + } + } + } + break; + } + case "elicitation": { + const eCfg = cfg as ElicitationStep; + eCfg.message = normalizeString(step.id, eCfg.message, "message"); + break; + } + // transform and conditional use raw JS — do NOT normalize templates, + // but DO strip legacy .result references + case "transform": { + const tCfg = cfg as TransformStep; + const stripped = tCfg.expression.replace( + /\bsteps\.(\w+)\.result\b/g, + "steps.$1", + ); + if (stripped !== tCfg.expression) { + warnings.push({ + stepId: step.id, + code: "FIELD_STRIPPED", + message: `Auto-stripped legacy .result from transform expression: "${tCfg.expression}" → "${stripped}"`, + }); + tCfg.expression = stripped; + } + break; + } + case "conditional": { + const cCfg = cfg as ConditionalStep; + const stripped = cCfg.condition.replace( + /\bsteps\.(\w+)\.result\b/g, + "steps.$1", + ); + if (stripped !== cCfg.condition) { + warnings.push({ + stepId: step.id, + code: "FIELD_STRIPPED", + message: `Auto-stripped legacy .result from conditional: "${cCfg.condition}" → "${stripped}"`, + }); + cCfg.condition = stripped; + } + break; + } + } + } + + return warnings; +} + +export function flattenNestedSteps(steps: ComposeStepInput[]): ComposerWarning[] { + const warnings: ComposerWarning[] = []; + const extracted: ComposeStepInput[] = []; + + for (const step of steps) { + const cfg = step.config as unknown as Record; + for (const key of ["steps", "subSteps"] as const) { + const nested = cfg[key]; + if (!Array.isArray(nested)) continue; + for (const sub of nested) { + if (sub && typeof sub === "object" && sub.id) { + const subStep: ComposeStepInput = { + id: sub.id, + label: sub.label ?? sub.id, + config: sub.config ?? sub, + dependsOn: [step.id], + }; + extracted.push(subStep); + warnings.push({ + stepId: step.id, + code: "IMPLICIT_DEP_ADDED", + message: `Flattened nested step "${sub.id}" from "${step.id}.${key}" to top-level with dependsOn: ["${step.id}"]`, + }); + } + } + delete cfg[key]; + } + } + + steps.push(...extracted); + return warnings; +} + From 00aef397f8b00252f724a7e59b9462998dfa359b Mon Sep 17 00:00:00 2001 From: sezallagwal Date: Tue, 7 Jul 2026 19:06:47 +0530 Subject: [PATCH 2/2] fix(composer): scope forEach aliases to their owning step Aliases from `forEach ... as ` were normalized globally, so an out-of-scope `{{x.field}}` in an unrelated step was silently rewritten to `{{steps.x.field}}` and resolved to empty at runtime. Scope alias rewriting to the owning step's inputMapping, treat the forEach collection expression as out of scope, and reject out-of-scope alias references with a clear ComposerError. Also fix pre-existing lint issues in the file and add unit tests covering the scoping behavior. --- src/composer/normalization.ts | 121 ++++++++--- .../normalization-alias-scope.unit.test.ts | 191 ++++++++++++++++++ 2 files changed, 280 insertions(+), 32 deletions(-) create mode 100644 src/tests/composer/normalization-alias-scope.unit.test.ts diff --git a/src/composer/normalization.ts b/src/composer/normalization.ts index 70482bf..1c34340 100644 --- a/src/composer/normalization.ts +++ b/src/composer/normalization.ts @@ -7,9 +7,15 @@ import type { SamplingStep, TransformStep, } from "../workflow/types.js"; -import { ComposerError, type ComposerWarning, type ComposeStepInput } from "./types.js"; +import { + ComposerError, + type ComposerWarning, + type ComposeStepInput, +} from "./types.js"; -export function normalizeStepFields(steps: ComposeStepInput[]): ComposerWarning[] { +export function normalizeStepFields( + steps: ComposeStepInput[], +): ComposerWarning[] { const warnings: ComposerWarning[] = []; for (const step of steps) { const cfg = step.config as unknown as Record; @@ -35,7 +41,6 @@ export function normalizeStepFields(steps: ComposeStepInput[]): ComposerWarning[ return warnings; } - export function normalizeEventParamShorthand( steps: ComposeStepInput[], params: JSONSchema7, @@ -44,7 +49,13 @@ export function normalizeEventParamShorthand( const paramProps = params.properties ? Object.keys(params.properties) : []; if (paramProps.length === 0) return warnings; - // Collect forEach iteration variable names so we don't rewrite them as params + // Reserve every forEach loop-variable name so it is never rewritten as an + // event param. A name that is declared as a `forEach ... as ` always + // wins over a same-named param: references to it are loop variables, not + // params. Reserving them globally (rather than per-step) is deliberate — it + // guarantees an out-of-scope alias reference cannot be silently "rescued" + // into a `params.*` reference here, so it still reaches the scope check in + // normalizeTemplateFields and fails there instead of resolving to empty. const forEachAsVars = new Set(); for (const step of steps) { if (step.config.type === "api_call" && (step.config as ApiCallStep).as) { @@ -136,7 +147,7 @@ export function normalizeEventParamShorthand( } function rewriteJs(stepId: string, value: string, fieldName: string): string { - let result = value; + const result = value; for (const rule of jsRewriters) { if (rule.re.test(result)) { warnings.push({ @@ -260,8 +271,8 @@ function convertHandlebarsBlocks( if (unsupportedBlock) { throw new ComposerError( `Step "${stepId}" field "${fieldName}" uses unsupported Handlebars helper "{{#${unsupportedBlock[1]}}}". ` + - `The template engine uses {{jsExpression}} syntax. ` + - `Use JavaScript expressions instead (e.g. array.map(), ternary operators).`, + `The template engine uses {{jsExpression}} syntax. ` + + `Use JavaScript expressions instead (e.g. array.map(), ternary operators).`, ); } @@ -274,8 +285,8 @@ function convertHandlebarsBlocks( if (/\{\{#(each|if)\b/.test(body)) { throw new ComposerError( `Step "${stepId}" field "${fieldName}" uses nested Handlebars blocks which cannot be auto-converted. ` + - `Use JavaScript expressions instead. Example: ` + - `{{${col}.map(item => item.name + ": " + item.value).join("\\n")}}`, + `Use JavaScript expressions instead. Example: ` + + `{{${col}.map(item => item.name + ": " + item.value).join("\\n")}}`, ); } @@ -316,8 +327,8 @@ function convertHandlebarsBlocks( } catch { throw new ComposerError( `Step "${stepId}" field "${fieldName}": auto-converted Handlebars {{#each}} failed to compile. ` + - `Original: "${_.trim()}". Converted: "${expr}". ` + - `Use JavaScript expressions directly instead.`, + `Original: "${_.trim()}". Converted: "${expr}". ` + + `Use JavaScript expressions directly instead.`, ); } @@ -379,20 +390,38 @@ function convertHandlebarsBlocks( return result; } -export function normalizeTemplateFields(steps: ComposeStepInput[]): ComposerWarning[] { +export function normalizeTemplateFields( + steps: ComposeStepInput[], +): ComposerWarning[] { const warnings: ComposerWarning[] = []; - const asVars = new Set(); + // A `forEach ... as ` declaration introduces a per-iteration loop + // variable. That variable ONLY exists while its owning step iterates, so it + // is only valid inside that step's own `inputMapping`. It must never leak + // into sibling/downstream steps: at runtime the alias is undefined there and + // silently resolves to empty. We therefore track every alias together with + // the id(s) of the step(s) that own it, so out-of-scope references can be + // rejected instead of blindly rewritten. + const aliasNames = new Set(); + const aliasOwners = new Map(); for (const step of steps) { if (step.config.type === "api_call" && (step.config as ApiCallStep).as) { - asVars.add((step.config as ApiCallStep).as!); + const alias = (step.config as ApiCallStep).as!; + aliasNames.add(alias); + const owners = aliasOwners.get(alias) ?? []; + owners.push(step.id); + aliasOwners.set(alias, owners); } } + // `scopedAlias` is the loop variable that is legal to reference in the field + // currently being normalized (undefined when no alias is in scope). Only the + // owning step's `inputMapping` passes a defined value. function normalizeString( stepId: string, value: string, fieldName: string, + scopedAlias?: string, ): string { // Collapse \n / \\n / \\\n → real newline (LLMs often multi-escape in JSON tool args) let result = value.replace(/\\+n/g, "\n").replace(/\\+t/g, "\t"); @@ -412,19 +441,38 @@ export function normalizeTemplateFields(steps: ComposeStepInput[]): ComposerWarn result = wrapped; } - for (const asVar of asVars) { + // forEach alias handling — strictly scoped to the owning step's inputMapping. + // Inside that scope the shorthand `{{alias.field}}` is rewritten to the + // canonical `{{steps.alias.field}}`. Anywhere else, a reference to a known + // alias is a scope violation and fails composition, because it would + // resolve to an empty value at runtime. + for (const alias of aliasNames) { const asRefRe = new RegExp( - `\\{\\{${asVar}\\.(\\w+(?:\\.\\w+)*)\\}\\}`, + `\\{\\{${alias}\\.(\\w+(?:\\.\\w+)*)\\}\\}`, "g", ); - const rewritten = result.replace(asRefRe, `{{steps.${asVar}.$1}}`); - if (rewritten !== result) { - warnings.push({ - stepId, - code: "AS_VAR_REWRITTEN", - message: `Rewritten as-variable reference in ${fieldName}: "${result}" → "${rewritten}"`, - }); - result = rewritten; + if (alias === scopedAlias) { + const rewritten = result.replace(asRefRe, `{{steps.${alias}.$1}}`); + if (rewritten !== result) { + warnings.push({ + stepId, + code: "AS_VAR_REWRITTEN", + message: `Rewritten as-variable reference in ${fieldName}: "${result}" → "${rewritten}"`, + }); + result = rewritten; + } + } else if (asRefRe.test(result)) { + const owners = aliasOwners.get(alias) ?? []; + const ownerHint = + owners.length > 0 + ? `step "${owners.join('", "')}"` + : "the step that declares it"; + throw new ComposerError( + `Step "${stepId}" references forEach alias "${alias}" in ${fieldName}, ` + + `but "${alias}" is a loop variable that only exists inside the inputMapping of ${ownerHint}. ` + + `A forEach alias cannot be used outside the step that declares it — the value would be empty at runtime. ` + + `Move this logic into the loop step, or reference the completed step result via "steps." instead.`, + ); } } @@ -446,10 +494,11 @@ export function normalizeTemplateFields(steps: ComposeStepInput[]): ComposerWarn stepId: string, value: unknown, fieldName: string, + scopedAlias?: string, ): unknown { if (typeof value === "string") { // Detect stringified JSON objects/arrays and parse them back to native types - if (/^\s*[\[{]/.test(value)) { + if (/^\s*[[{]/.test(value)) { try { const parsed = JSON.parse(value); if (typeof parsed === "object" && parsed !== null) { @@ -458,21 +507,23 @@ export function normalizeTemplateFields(steps: ComposeStepInput[]): ComposerWarn code: "STRINGIFIED_JSON_PARSED", message: `Auto-parsed stringified JSON in ${fieldName}: "${value.length > 60 ? value.slice(0, 60) + "..." : value}"`, }); - return normalizeValue(stepId, parsed, fieldName); + return normalizeValue(stepId, parsed, fieldName, scopedAlias); } - } catch { } + } catch { + // Not valid JSON — leave the string as-is for template normalization. + } } - return normalizeString(stepId, value, fieldName); + return normalizeString(stepId, value, fieldName, scopedAlias); } if (Array.isArray(value)) { return value.map((item, i) => - normalizeValue(stepId, item, `${fieldName}[${i}]`), + normalizeValue(stepId, item, `${fieldName}[${i}]`, scopedAlias), ); } if (typeof value === "object" && value !== null) { const result: Record = {}; for (const [k, v] of Object.entries(value)) { - result[k] = normalizeValue(stepId, v, `${fieldName}.${k}`); + result[k] = normalizeValue(stepId, v, `${fieldName}.${k}`, scopedAlias); } return result; } @@ -507,14 +558,19 @@ export function normalizeTemplateFields(steps: ComposeStepInput[]): ComposerWarn } } + // The loop alias is in scope only within THIS step's inputMapping. + const ownedAlias = apiCfg.forEach ? apiCfg.as : undefined; if (apiCfg.inputMapping) { apiCfg.inputMapping = normalizeValue( step.id, apiCfg.inputMapping, "inputMapping", + ownedAlias, ) as Record; } if (apiCfg.forEach) { + // `forEach` is the collection expression, evaluated before iteration + // begins — the loop alias does not exist yet, so it is NOT in scope here. apiCfg.forEach = normalizeString(step.id, apiCfg.forEach, "forEach"); } break; @@ -583,7 +639,9 @@ export function normalizeTemplateFields(steps: ComposeStepInput[]): ComposerWarn return warnings; } -export function flattenNestedSteps(steps: ComposeStepInput[]): ComposerWarning[] { +export function flattenNestedSteps( + steps: ComposeStepInput[], +): ComposerWarning[] { const warnings: ComposerWarning[] = []; const extracted: ComposeStepInput[] = []; @@ -615,4 +673,3 @@ export function flattenNestedSteps(steps: ComposeStepInput[]): ComposerWarning[] steps.push(...extracted); return warnings; } - diff --git a/src/tests/composer/normalization-alias-scope.unit.test.ts b/src/tests/composer/normalization-alias-scope.unit.test.ts new file mode 100644 index 0000000..d726ec6 --- /dev/null +++ b/src/tests/composer/normalization-alias-scope.unit.test.ts @@ -0,0 +1,191 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import type { JSONSchema7 } from "json-schema"; +import { + normalizeEventParamShorthand, + normalizeTemplateFields, +} from "../../composer/normalization.js"; +import { ComposerError, type ComposeStepInput } from "../../composer/types.js"; +import type { ApiCallStep } from "../../workflow/types.js"; + +function apiStep( + id: string, + config: Partial & { inputMapping: Record }, +): ComposeStepInput { + return { + id, + label: id, + config: { + type: "api_call", + operationId: `op_${id}`, + ...config, + } as ApiCallStep, + }; +} + +describe("normalizeTemplateFields - forEach alias scoping", () => { + it("rewrites an alias reference inside the owning step's inputMapping", () => { + const steps: ComposeStepInput[] = [ + apiStep("send", { + forEach: "{{steps.list.channels}}", + as: "ch", + inputMapping: { roomId: "{{ch.name}}" }, + }), + ]; + + const warnings = normalizeTemplateFields(steps); + + const cfg = steps[0].config as ApiCallStep; + assert.equal(cfg.inputMapping.roomId, "{{steps.ch.name}}"); + assert.ok( + warnings.some((w) => w.code === "AS_VAR_REWRITTEN"), + "expected an AS_VAR_REWRITTEN warning for the in-scope rewrite", + ); + }); + + it("throws when a non-owning step references the alias", () => { + const steps: ComposeStepInput[] = [ + apiStep("send", { + forEach: "{{steps.list.channels}}", + as: "ch", + inputMapping: { roomId: "{{ch.name}}" }, + }), + apiStep("notify", { + // "ch" is out of scope here — it belongs to the "send" loop. + inputMapping: { text: "posted to {{ch.name}}" }, + }), + ]; + + assert.throws( + () => normalizeTemplateFields(steps), + (err: unknown) => { + assert.ok(err instanceof ComposerError); + assert.match(err.message, /forEach alias "ch"/); + assert.match(err.message, /notify/); + return true; + }, + ); + }); + + it("throws when the owning step uses its alias in the forEach collection expression", () => { + const steps: ComposeStepInput[] = [ + apiStep("send", { + // The alias does not exist yet while the collection is being resolved. + forEach: "{{ch.items}}", + as: "ch", + inputMapping: { roomId: "static" }, + }), + ]; + + assert.throws( + () => normalizeTemplateFields(steps), + (err: unknown) => + err instanceof ComposerError && /forEach alias "ch"/.test(err.message), + ); + }); + + it("throws when a step references another loop's alias inside its own inputMapping", () => { + const steps: ComposeStepInput[] = [ + apiStep("outer", { + forEach: "{{steps.a.list}}", + as: "outerItem", + inputMapping: { id: "{{outerItem.id}}" }, + }), + apiStep("inner", { + forEach: "{{steps.b.list}}", + as: "innerItem", + // References "outerItem" which belongs to the "outer" step. + inputMapping: { id: "{{innerItem.id}}", parent: "{{outerItem.id}}" }, + }), + ]; + + assert.throws( + () => normalizeTemplateFields(steps), + (err: unknown) => + err instanceof ComposerError && + /forEach alias "outerItem"/.test(err.message), + ); + }); + + it("leaves canonical steps. references untouched and does not throw", () => { + const steps: ComposeStepInput[] = [ + apiStep("send", { + forEach: "{{steps.list.channels}}", + as: "ch", + inputMapping: { roomId: "{{steps.ch.name}}" }, + }), + ]; + + const warnings = normalizeTemplateFields(steps); + + const cfg = steps[0].config as ApiCallStep; + assert.equal(cfg.inputMapping.roomId, "{{steps.ch.name}}"); + assert.ok(!warnings.some((w) => w.code === "AS_VAR_REWRITTEN")); + }); + + it("scopes each alias independently so identical rewrites happen only in their own step", () => { + const steps: ComposeStepInput[] = [ + apiStep("first", { + forEach: "{{steps.a.rows}}", + as: "row", + inputMapping: { v: "{{row.value}}" }, + }), + apiStep("second", { + forEach: "{{steps.b.rows}}", + as: "row", + inputMapping: { v: "{{row.value}}" }, + }), + ]; + + const warnings = normalizeTemplateFields(steps); + + assert.equal( + (steps[0].config as ApiCallStep).inputMapping.v, + "{{steps.row.value}}", + ); + assert.equal( + (steps[1].config as ApiCallStep).inputMapping.v, + "{{steps.row.value}}", + ); + assert.equal( + warnings.filter((w) => w.code === "AS_VAR_REWRITTEN").length, + 2, + ); + }); + + it("keeps alias precedence: an out-of-scope ref is never rescued into a param", () => { + // A forEach alias intentionally shares its name with a workflow param. + // The event-param normalizer must NOT rewrite the out-of-scope reference + // into "params.channel.*"; it must remain an alias reference so the scope + // check rejects it instead of silently resolving to an empty value. + const params: JSONSchema7 = { + type: "object", + properties: { channel: { type: "object" } }, + }; + const steps: ComposeStepInput[] = [ + apiStep("loop", { + forEach: "{{steps.list.channels}}", + as: "channel", + inputMapping: { roomId: "{{channel.id}}" }, + }), + apiStep("after", { + inputMapping: { roomId: "{{channel.id}}" }, + }), + ]; + + // Runs first in the pipeline; must leave the alias references untouched. + normalizeEventParamShorthand(steps, params); + assert.equal( + (steps[1].config as ApiCallStep).inputMapping.roomId, + "{{channel.id}}", + "event-param normalizer must not rewrite an alias-named reference to params.*", + ); + + assert.throws( + () => normalizeTemplateFields(steps), + (err: unknown) => + err instanceof ComposerError && + /forEach alias "channel"/.test(err.message), + ); + }); +});