From e3c08209bb4d2e0caae903647df3746d7bcd9a35 Mon Sep 17 00:00:00 2001 From: Mubashir Rahim Date: Wed, 3 Jun 2026 14:26:26 +0500 Subject: [PATCH] fix(workflow): detect duplicate trigger paths when first node has no id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validate_trigger_paths used seen_paths.get(trigger_path) and treated a None result as "path not seen yet". But None is also what node.get("id") returns for a node without an id, so when the first trigger node sharing a path had no id, it was stored as None and every later node with the same path was silently accepted as unique — duplicate trigger paths slipped through validation. Use a membership test (trigger_path not in seen_paths) so "first occurrence" and "node_id happens to be None" are no longer conflated. Behavior is unchanged for nodes that have ids. Adds a regression test that fails before and passes after. Co-Authored-By: Claude Opus 4.8 --- api/services/workflow/trigger_paths.py | 3 +- api/tests/test_trigger_path_validation.py | 34 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/api/services/workflow/trigger_paths.py b/api/services/workflow/trigger_paths.py index ed34345b..f53f6934 100644 --- a/api/services/workflow/trigger_paths.py +++ b/api/services/workflow/trigger_paths.py @@ -127,8 +127,7 @@ def validate_trigger_paths( ) ) - first_node_id = seen_paths.get(trigger_path) - if first_node_id is None: + if trigger_path not in seen_paths: seen_paths[trigger_path] = node_id else: issues.append( diff --git a/api/tests/test_trigger_path_validation.py b/api/tests/test_trigger_path_validation.py index 758eceb4..1b828450 100644 --- a/api/tests/test_trigger_path_validation.py +++ b/api/tests/test_trigger_path_validation.py @@ -54,3 +54,37 @@ def test_validate_trigger_paths_rejects_long_and_duplicate_paths(): in messages ) assert "Trigger path is duplicated in this workflow." in messages + + +def test_validate_trigger_paths_detects_duplicate_when_first_node_has_no_id(): + """A duplicate trigger path must be flagged even when the first node sharing + that path has no ``id`` (``node.get("id")`` is None). + + Regression: the duplicate check previously used ``seen_paths.get(path)`` and + treated a ``None`` result as "not seen yet", so a first node with a missing + id (stored as None) made every later node with the same path slip through + undetected. + """ + workflow_definition = { + "nodes": [ + # No "id" key -> node_id resolves to None. + {"type": "trigger", "data": {"trigger_path": "sales_agent"}}, + { + "id": "trigger-2", + "type": "trigger", + "data": {"trigger_path": "sales_agent"}, + }, + ], + "edges": [], + } + + issues = validate_trigger_paths(workflow_definition) + messages = [issue.message for issue in issues] + + assert "Trigger path is duplicated in this workflow." in messages + duplicate_issue = next( + issue + for issue in issues + if issue.message == "Trigger path is duplicated in this workflow." + ) + assert duplicate_issue.node_id == "trigger-2"