Conversation
…lows Increase TARGET_COUNT from 100 to 1000, candidate threshold from 200 to 2000, and max search pages from 10 to 20 to fetch more published workflows from n8n.io template library. Updates the zip fixture with 1455 total workflows for broader test coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add validated expectedWarnings to manifest by cross-checking codegen warnings against original workflow validation (only real warnings) - Skip workflows with known codegen issues (syntax errors, node count mismatches, structural roundtrip failures) - Normalize null connection slots to [] in real-workflow test comparisons - Handle undefined typeVersion in original workflows (SDK defaults to 1) - Add update-expected-warnings script for future manifest updates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address multiple codegen issues that caused test fixtures to be skipped: - Quote string sticky note colors (e.g., 'white') in generated code - Guard against undefined typeVersion in codegen output - Handle duplicate node names in semantic graph and workflow-import - Recognize SwitchCaseBuilder targets in disconnected-node validator - Resolve orphaned subnodes to nearest parent when AI connections reference non-existent nodes - Add manifest-level skip support for workflows with malformed data Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…asons Move workflow skip declarations from test-level SKIP_WORKFLOWS sets to manifest.json with skip + skipReason fields for better traceability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix multiple root causes to un-skip workflows 11128, 4808, 5288, 4028, 8055, 4910, 10440, 11807: - Non-ASCII variable names: Add hashFallbackName() for names that produce empty strings after stripping non-ASCII chars (e.g. Japanese, Chinese, slash-only names) - typeVersion normalization: Convert string typeVersions to numbers in both semantic-graph and workflow-import paths - Flat array connections: Add normalizeConnections() to handle legacy tuple format [node, type, index] alongside object format - Expression newline normalization: Apply escapeNewlinesInExpressionStrings in test comparison so raw newlines in JS string literals are normalized identically in both original and roundtripped parameters Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ow, and subnode collisions - Detect and correct reversed AI connections (parent→subnode instead of subnode→parent) using node type pattern matching (un-skips workflow 7643) - Add depth guard to addBranchToGraph() to prevent stack overflow in deeply nested if-else chains (un-skips workflow 4012) - Skip dual-role nodes in subnode collection to prevent duplicate declarations when a node is both a subnode and a main flow node - Add depth guard to AST interpreter evaluate() as safety net - Disambiguate AI subnode routing for duplicate node names using position proximity - Filter AI-only connection keys from roundtrip comparison Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dk codegen Dispatch .onError() composite targets (IfElseBuilder, SwitchCaseBuilder) in addSingleNodeConnectionTargets instead of skipping them. Add addMissingCompositeTargets() in toJSON() to catch composites missed during chain processing, using a WeakSet to track already-dispatched composites and prevent infinite recursion. Also parse error type connections in semantic graph, fix circular subnode references in code-generator, and normalize placeholder values in roundtrip test. Un-skips workflows 5929, 5900, 3820, 9473, 5979, 5370, 11027. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mutdmour
changed the title
This comment has been minimized.
This comment has been minimized.
…kflows
Sticky notes with object-type color values (e.g. `{}`) produced
`[object Object]` in generated code, causing SyntaxError. Now only
number and string color values are serialized; invalid types are omitted.
Also normalize empty object colors in roundtrip test comparisons.
Un-skips workflows: 4022, 3288
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…test fixtures MultiOutput nodes (e.g. textClassifier) nested inside other multiOutput chains or deferred merge downstreams were missing their .output(n).to() calls in generated code. This caused node loss during roundtrip for workflows with deeply nested classifier/router patterns. Also handles null color values in sticky note parameter normalization. Un-skips: 5774, 8044, 2978, 5392, 7751, 7364, 3762, 2468 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s, un-skip 3 workflows When multiple triggers fan out to the same targets that converge at a merge node, the second trigger's connections were lost because its branch targets were already visited. Now creates varRef nodes for already-visited branches so the source→branch connections are preserved. Un-skips: 5042, 2986, 9881 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s, un-skip 2 workflows When a node fans out to targets including an already-visited merge node, the deferred connection to the merge was missing, causing it to receive both inputs on the same index and triggering MERGE_SINGLE_INPUT warnings. Now creates deferred connections with the correct input index for merge-type branches that are already visited in the detectMergePattern handler. Un-skips: 4627, 4918 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…name handling, un-skip 4 workflows Recursively process connection targets after addNodeWithSubnodes to extract nodes from nested .onError()/.onCase() chains. Redistribute connections and AI subnodes for duplicate node names using position proximity. Fix code generator to use semantic graph keys instead of original JSON names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add tests for normalizeConnections, error connections in semantic graph, duplicate node names, reversed AI connections, flat tuple normalization, hasErrorOutput with error connections, and workflow-import edge cases. Fix unnecessary type assertions flagged by eslint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Reversed AI connections (parent→subnode stored as source) were being silently corrected during codegen roundtrip, which masked data issues and prevented full AI connection comparison in roundtrip tests. - Remove isReversedAiConnection() and matchesSubnodePatternForConnection() - Remove reversed connection detection block in parseAiConnections() - Compare all connection keys (including ai_*) in roundtrip tests - Skip workflow 7643 which has reversed AI connections in source JSON Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ith index-based routing Use target.index from workflow JSON to route AI subnodes instead of canvas position proximity. This removes the fragile findNearestParent heuristic and correctly handles multiple subnodes of the same type (e.g., primary + fallback language model) by preserving index ordering. - Add index field to SubnodeConnection type - Delete isSubnodeType, AI_CONNECTION_SUBNODE_PATTERNS, findNearestParent - Rewrite parseAiConnections to use target.index directly - Sort subnodes by index after graph construction - Propagate array position as index in builder serialization - Add ai_document, ai_embedding, ai_reranker to AI_OPTIONAL_ARRAY_TYPES Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
…_WORKFLOWS from roundtrip tests Skip logic is handled via manifest.json skip/skipReason fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mutdmour
commented
Feb 23, 2026
| * Remaining patterns: merge VarRef leaks in SIB/composite downstream chains, | ||
| * multi-trigger fan-out ordering, and missing connections in complex convergence. */ | ||
| // prettier-ignore | ||
| const SKIP_DEEP_CONNECTION_CHECK = new Set<string>([ |
Contributor
Author
There was a problem hiding this comment.
will address in next PR
mutdmour
requested review from
a team and
aalises
and removed request for
a team
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sertions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
aalises
reviewed
Feb 23, 2026
packages/@n8n/workflow-sdk/src/workflow-builder/subnode-utils.ts
Outdated
Show resolved
Hide resolved
…-fetch-1000-workflows
…-fetch-1000-workflows # Conflicts: # packages/@n8n/workflow-sdk/src/workflow-builder.test.ts # packages/@n8n/workflow-sdk/src/workflow-builder.ts
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 22 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/@n8n/workflow-sdk/src/workflow-builder/node-builders/node-builder.ts">
<violation number="1" location="packages/@n8n/workflow-sdk/src/workflow-builder/node-builders/node-builder.ts:480">
P2: Unsafe `as T` cast discards the `InputTarget` type — update the `NodeInstance` interface instead.
The `NodeInstance.onError` interface in `base.ts` declares `handler: T` but the implementation (`NodeInstanceImpl`) now accepts `T | InputTarget`. This chain method works around the mismatch with `as T`, which silently drops the `InputTarget` case from type-checking. The proper fix is to widen the interface signature to `T | InputTarget` and remove this cast.
(Based on your team's feedback about removing unnecessary TypeScript casts.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/@n8n/workflow-sdk/src/codegen/code-generator.ts">
<violation number="1" location="packages/@n8n/workflow-sdk/src/codegen/code-generator.ts:746">
P2: `collectNestedMultiOutputs` does not recurse into `ifElse.errorHandler`. Since this PR adds error handler support on `IfElseCompositeNode`, if the error handler composite contains a nested `multiOutput` node its output connections will be silently dropped from the generated code. The `ifElse` branch in `collectNestedMultiOutputs` should also traverse `errorHandler`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/@n8n/workflow-sdk/src/workflow-builder/node-builders/node-builder.ts
Show resolved
Hide resolved
burivuhster
approved these changes
Feb 26, 2026
Contributor
burivuhster
left a comment
burivuhster
left a comment
There was a problem hiding this comment.
Looks good! Ignore comments if not relevant
packages/@n8n/workflow-sdk/src/workflow-builder/node-builders/node-builder.ts
Show resolved
Hide resolved
Merged
Tuukkaa
pushed a commit
that referenced
this pull request
Mar 2, 2026
…ows and fix codegen bugs (#26041) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Got released with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.onError()handlers, and cycle detection in subnode collectionChanges
Test fixtures expansion
update-expected-warnings.tsandvalidate-expected-warnings.tsscriptsCodegen bug fixes
errortype connections in semantic graph and check them inhasErrorOutput().onError()composite targets (IfElseBuilder, SwitchCaseBuilder) inaddSingleNodeConnectionTargetsinstead of skipping themaddMissingCompositeTargets()intoJSON()to catch composites missed during chain processing, using a WeakSet to prevent infinite recursioncollectSubnodesAsVariablesand skip self-referencing subnodes[node, type, index]connection formatTest plan
pnpm testinpackages/@n8n/workflow-sdk— all 11,324 tests passpnpm typecheckinpackages/@n8n/workflow-sdk— clean🤖 Generated with Claude Code