fix(n8n): three real bugs surfaced dogfooding the n8n parser on 308 real workflows#56
Merged
Merged
Conversation
…ources n8n attaches an AI Agent's sub-resources (model / tool / memory / output parser) via ai_* connections whose SOURCE is the sub-resource and whose target is the Agent, so the parser emits sub-resource -> Agent edges. The trigger reaches the Agent through the main chain, but nothing flows INTO a sub-resource, so reachability flagged every model/tool/memory node as unreachable_node. On real langchain-heavy n8n workflows this fired on nearly every AI node (3-node trigger->Agent + ChatModel repro emits a 100%- confidence FP). Fix: treat ai_* edges as UNDIRECTED for reachability only -- a sub-resource of a reachable Agent is reachable (the Agent invokes it at runtime). The directed graph that cycle_detection consumes is left untouched: a naive bidirectional edge in the PARSER instead converts the unreachable FP into a cycle_detection FP (Agent<->sub-resource 2-cycle), so the fix lives in the reachability rule, not the parser. Tests: AISubResourceReachableViaAgent (sub-resources of a reachable Agent -> 0 findings) + AISubResourceUnreachableIfAgentIsolated (the undirected hop does not leak: an isolated Agent and its sub-resource stay unreachable). go test ./domain/rules/... ./infrastructure/parser/... green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cycle_detection reported a DIFFERENT node of a multi-node cycle on each run of the same workflow. Two map-iteration sources fed the DFS: the isolated- cycle start loop (`for id := range graph.Nodes`) and the per-node outgoing walk (OutgoingEdges follows the parser's edge-construction order, which for the n8n parser is itself Go map iteration). The DFS visit order decides which node is seen as the back-edge target, so the reported Critical node was non-deterministic. Measured on 308 real n8n workflows (enescingoz/awesome-n8n-templates): total cycle_detection findings flipped between 62 and 63 across runs, and 4 specific workflows reported a different node each run (e.g. "AI Youtube Trend Finder" cycled through if_longer_than_3_ / loop_over_items1 / find_video_data1). After sorting both the isolated-start order and the outgoing-edge order inside the detector: 6/6 runs == 62, 0 flaky files. The fix is contained in the rule (the parser's edge order is left as-is), so no fixtures change. This matters for the determinism guarantee the rule advertises: a static analyzer that flags a different node each run cannot be put in CI. Test: DeterministicIsolatedCycle (50 repeat analyses of an isolated 3-node cycle must report the same, sorted-first node). go test ./domain/rules/... ./infrastructure/parser/... green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
…ion) @n8n/n8n-nodes-langchain.chatTrigger and manualChatTrigger contain the substring "n8n-nodes-langchain", so the broad `langchain → LLM` rule in mapN8nNodeType matched them BEFORE the trigger rule. They lost their Config["category"]="trigger", were never collected by allTriggerNodeIDs, and so were never picked as the entry node. In an AI chat workflow the chatTrigger is usually the ONLY entry, so the entire downstream graph (Agent + model + memory + tools) was reported as unreachable_node. Fix: classify triggers/webhooks at the TOP of mapN8nNodeType, before the langchain→LLM rule (the old trailing trigger check is now dead code and is removed). Anything whose lowercased type contains "trigger"/"webhook" is a trigger; no real langchain LLM node contains those substrings. Measured on 308 real n8n workflows, unreachable_node findings: 973 (pre-fixes) -> 444 (ai_* undirected) -> 198 (this fix) = -80% total. Test: ChatTriggerIsEntry (chatTrigger is a trigger Tool AND the chosen entry node). go test ./domain/rules/... ./infrastructure/parser/... green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
hatyibei
changed the title
This was referenced Jun 19, 2026
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
1 participant
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.
Dogfooding the n8n parser against 308 real workflows (
enescingoz/awesome-n8n-templates) surfaced three pre-existing correctness bugs in the langchain-AI path. All reproduce onnpx shingan-lint demo-grade inputs and all undermine "runshingan analyzeon your own graph" as a credible CTA — so they're correctness, not features.unreachable_nodeacross the three commits: 973 → 444 → 198 (−80%).Bug 1 — systematic
unreachable_nodeFP on AI sub-resources (4cb6eca)n8n attaches an AI Agent's sub-resources (model / tool / memory / output parser) via
ai_*connections whose source is the sub-resource, so the parser emitssub-resource → Agentedges. The trigger reaches the Agent viamain, but nothing flows into the sub-resource → every model/tool/memory node is flagged unreachable.ai_*edges as undirected for reachability only. A naive bidirectional edge in the parser would instead turn the FP into acycle_detection2-cycle FP, so the fix lives in the rule.Bug 2 —
cycle_detectionnon-determinism (6d85a1d)The same workflow reported a different node of a multi-node cycle each run. Two map-iteration sources fed the DFS visit order, which decides the back-edge target.
if_longer_than_3_/loop_over_items1/find_video_data1).Bug 3 — chatTrigger mis-typed as LLM, orphaning chat workflows (cfcf181)
@n8n/n8n-nodes-langchain.chatTrigger/manualChatTriggercontain"n8n-nodes-langchain", so the broadlangchain → LLMrule matched them before the trigger rule. They lostcategory="trigger", were never collected as triggers, and were never picked as the entry — so in an AI chat workflow (where the chatTrigger is usually the only entry) the whole downstream graph was unreachable.mapN8nNodeType, before the langchain rule (old trailing check removed as dead code).Tests
AISubResourceReachableViaAgent/…UnreachableIfAgentIsolatedDeterministicIsolatedCycle(50 repeat analyses → stable, sorted-first node)ChatTriggerIsEntry(chatTrigger is a trigger Tool AND the chosen entry)go test ./domain/rules/... ./infrastructure/parser/...greenNot addressed here (follow-ups)
unreachable_nodeFP pattern (main-flow subgraph not traversed, 125 of the residual 198) — tracked in n8n: main-flow subgraph reported as unreachable_node (residual FP after #56) #57eval_missing160 hits: 152 are n8ncodenodes (fixed scripts processing LLM output, weak threat), 5set(FP), only 3executeCommandare real candidates — unverifiederror_handler_checkerfiring on trigger nodes🤖 Generated with Claude Code