Fix/duplicate behavior tree id registration

include/behaviortree_cpp/bt_factory.h

@@ -316,11 +316,20 @@ class BehaviorTreeFactory
      *
      * where "tree_id" come from the XML attribute <BehaviorTree ID="tree_id">
      *
+     * @throws RuntimeError if the XML registers a BehaviorTree ID that is
+     *         already known to this factory. Call clearRegisteredBehaviorTrees()
+     *         first to replace previous definitions intentionally.
      */
   void registerBehaviorTreeFromFile(const std::filesystem::path& filename);
 
-  /// Same of registerBehaviorTreeFromFile, but passing the XML text,
-  /// instead of the filename.
+  /**
+     * @brief Same as registerBehaviorTreeFromFile, but passing the XML text
+     *        instead of the filename.
+     *
+     * @throws RuntimeError if the XML registers a BehaviorTree ID that is
+     *         already known to this factory. Call clearRegisteredBehaviorTrees()
+     *         first to replace previous definitions intentionally.
+     */
   void registerBehaviorTreeFromText(const std::string& xml_text);
 
   /// Returns the ID of the trees registered either with
@@ -446,6 +455,10 @@ class BehaviorTreeFactory
    * @param text        string containing the XML
    * @param blackboard  blackboard of the root tree
    * @return the newly created tree
+   *
+   * @throws RuntimeError if the XML registers a BehaviorTree ID that is
+   *         already known to this factory. Call clearRegisteredBehaviorTrees()
+   *         first to replace previous definitions intentionally.
    */
   [[nodiscard]] Tree createTreeFromText(
       const std::string& text, Blackboard::Ptr blackboard = Blackboard::create());
@@ -460,11 +473,25 @@ class BehaviorTreeFactory
    * @param file_path   location of the file to load
    * @param blackboard  blackboard of the root tree
    * @return the newly created tree
+   *
+   * @throws RuntimeError if the XML registers a BehaviorTree ID that is
+   *         already known to this factory. Call clearRegisteredBehaviorTrees()
+   *         first to replace previous definitions intentionally.
    */
   [[nodiscard]] Tree
   createTreeFromFile(const std::filesystem::path& file_path,
                      Blackboard::Ptr blackboard = Blackboard::create());
 
+  /**
+   * @brief createTree instantiates a tree by ID from definitions already
+   *        present in the factory's parser. It does not load or parse XML;
+   *        duplicate-ID errors come from registerBehaviorTreeFrom* or
+   *        createTreeFrom*, not from createTree().
+   *
+   * @param tree_name   ID of the tree to instantiate
+   * @param blackboard  blackboard of the root tree
+   * @return the newly created tree
+   */
   [[nodiscard]] Tree createTree(const std::string& tree_name,
                                 Blackboard::Ptr blackboard = Blackboard::create());
 

src/xml_parsing.cpp

@@ -48,6 +48,7 @@
 
 #include <filesystem>
 #include <map>
+#include <unordered_map>
 
 #ifdef USING_ROS2
 #include <ament_index_cpp/get_package_share_directory.hpp>
@@ -243,10 +244,12 @@ struct XMLParser::PImpl
 
   std::list<std::unique_ptr<XMLDocument> > opened_documents;
   std::map<std::string, const XMLElement*> tree_roots;
+  std::unordered_map<std::string, std::string> tree_sources;
 
   const BehaviorTreeFactory* factory = nullptr;
 
   std::filesystem::path current_path;
+  std::string current_file = "<unknown>";
   std::map<std::string, SubtreeModel> subtree_models;
 
   int suffix_count = 0;
@@ -261,6 +264,8 @@ struct XMLParser::PImpl
     current_path = std::filesystem::current_path();
     opened_documents.clear();
     tree_roots.clear();
+    tree_sources.clear();
+    current_file = "<unknown>";
   }
 
 private:
@@ -294,6 +299,7 @@ void XMLParser::loadFromFile(const std::filesystem::path& filepath, bool add_inc
   doc->LoadFile(filepath.string().c_str());
 
   _p->current_path = std::filesystem::absolute(filepath.parent_path());
+  _p->current_file = std::filesystem::absolute(filepath).string();
 
   _p->loadDocImpl(doc, add_includes);
 }
@@ -305,6 +311,8 @@ void XMLParser::loadFromText(const std::string& xml_text, bool add_includes)
   XMLDocument* doc = _p->opened_documents.back().get();
   doc->Parse(xml_text.c_str(), xml_text.size());
 
+  _p->current_file = "<inline XML>";
+
   _p->loadDocImpl(doc, add_includes);
 }
 
@@ -418,13 +426,16 @@ void XMLParser::PImpl::loadDocImpl(XMLDocument* doc, bool add_includes)
 
     // change current path to the included file for handling additional relative paths
     const auto previous_path = current_path;
+    const std::string previous_file = current_file;
     current_path = std::filesystem::absolute(file_path.parent_path());
+    current_file = std::filesystem::absolute(file_path).string();
 
     next_doc->LoadFile(file_path.string().c_str());
     loadDocImpl(next_doc, add_includes);
 
     // reset current path to the previous value
     current_path = previous_path;
+    current_file = previous_file;
   }
 
   // Collect the names of all nodes registered with the behavior tree factory
@@ -457,7 +468,17 @@ void XMLParser::PImpl::loadDocImpl(XMLDocument* doc, bool add_includes)
       tree_name = "BehaviorTree_" + std::to_string(suffix_count++);
     }
 
+    const auto existing = tree_sources.find(tree_name);
+    if(existing != tree_sources.end())
+    {
+      throw RuntimeError("Duplicate BehaviorTree ID '", tree_name,
+                         "': first registered from '", existing->second,
+                         "', re-registered from '", current_file,
+                         "'. Call clearRegisteredBehaviorTrees() if you "
+                         "intend to reload.");
+    }
     tree_roots[tree_name] = bt_node;
+    tree_sources[tree_name] = current_file;
   }
 }
 

tests/CMakeLists.txt

@@ -23,6 +23,7 @@ set(BT_TESTS
   gtest_blackboard.cpp
   gtest_coroutines.cpp
   gtest_decorator.cpp
+  gtest_duplicate_tree.cpp
   gtest_enums.cpp
   gtest_factory.cpp
   gtest_fallback.cpp

tests/gtest_duplicate_tree.cpp

@@ -0,0 +1,218 @@
+/* Copyright (C) 2026 - duplicate BehaviorTree ID registration tests */
+
+#include "behaviortree_cpp/bt_factory.h"
+
+#include <chrono>
+#include <filesystem>
+#include <fstream>
+#include <string>
+
+#include <gtest/gtest.h>
+
+using namespace BT;
+
+namespace
+{
+class TempSubdir
+{
+public:
+  TempSubdir()
+  {
+    const auto stamp = std::chrono::steady_clock::now().time_since_epoch().count();
+    path = std::filesystem::temp_directory_path() /
+           ("bt_duplicate_tree_" + std::to_string(stamp));
+    std::filesystem::create_directories(path);
+  }
+
+  ~TempSubdir()
+  {
+    std::error_code ec;
+    std::filesystem::remove_all(path, ec);
+  }
+
+  std::filesystem::path path;
+
+  TempSubdir(const TempSubdir&) = delete;
+  TempSubdir& operator=(const TempSubdir&) = delete;
+};
+
+void writeFile(const std::filesystem::path& p, const std::string& content)
+{
+  std::ofstream out(p.string());
+  out << content;
+}
+
+std::string minimalTreeXml(const std::string& tree_id)
+{
+  return std::string(R"(
+    <root BTCPP_format="4">
+      <BehaviorTree ID=")" +
+                     tree_id + R"(">
+        <AlwaysSuccess/>
+      </BehaviorTree>
+    </root>
+  )");
+}
+
+}  // namespace
+
+TEST(DuplicateBehaviorTreeId, DuplicateIdAcrossFiles)
+{
+  TempSubdir dir;
+  const auto path_a = dir.path / "a.xml";
+  const auto path_b = dir.path / "b.xml";
+  writeFile(path_a, minimalTreeXml("DupTree"));
+  writeFile(path_b, minimalTreeXml("DupTree"));
+
+  const std::string abs_a = std::filesystem::absolute(path_a).string();
+  const std::string abs_b = std::filesystem::absolute(path_b).string();
+
+  BehaviorTreeFactory factory;
+  ASSERT_NO_THROW(factory.registerBehaviorTreeFromFile(path_a));
+
+  try
+  {
+    factory.registerBehaviorTreeFromFile(path_b);
+    FAIL() << "expected RuntimeError";
+  }
+  catch(const RuntimeError& e)
+  {
+    const std::string msg = e.what();
+    EXPECT_NE(msg.find(abs_a), std::string::npos) << msg;
+    EXPECT_NE(msg.find(abs_b), std::string::npos) << msg;
+  }
+}
+
+TEST(DuplicateBehaviorTreeId, DuplicateIdWithinSameText)
+{
+  const std::string xml = R"(
+<root BTCPP_format="4">
+  <BehaviorTree ID="foo"><AlwaysSuccess/></BehaviorTree>
+  <BehaviorTree ID="foo"><AlwaysFailure/></BehaviorTree>
+</root>
+)";
+
+  BehaviorTreeFactory factory;
+  try
+  {
+    factory.registerBehaviorTreeFromText(xml);
+    FAIL() << "expected RuntimeError";
+  }
+  catch(const RuntimeError& e)
+  {
+    const std::string msg = e.what();
+    EXPECT_NE(msg.find("<inline XML>"), std::string::npos) << msg;
+  }
+}
+
+TEST(DuplicateBehaviorTreeId, DuplicateIdAcrossFileAndText)
+{
+  TempSubdir dir;
+  const auto path = dir.path / "one.xml";
+  writeFile(path, minimalTreeXml("shared_id"));
+
+  const std::string abs_path = std::filesystem::absolute(path).string();
+
+  BehaviorTreeFactory factory;
+  ASSERT_NO_THROW(factory.registerBehaviorTreeFromFile(path));
+
+  const std::string xml = minimalTreeXml("shared_id");
+  try
+  {
+    factory.registerBehaviorTreeFromText(xml);
+    FAIL() << "expected RuntimeError";
+  }
+  catch(const RuntimeError& e)
+  {
+    const std::string msg = e.what();
+    EXPECT_NE(msg.find(abs_path), std::string::npos) << msg;
+    EXPECT_NE(msg.find("<inline XML>"), std::string::npos) << msg;
+  }
+}
+
+TEST(DuplicateBehaviorTreeId, DuplicateIdSamePathTwice)
+{
+  TempSubdir dir;
+  const auto path = dir.path / "reload.xml";
+  writeFile(path, minimalTreeXml("same"));
+
+  const std::string abs_path = std::filesystem::absolute(path).string();
+
+  BehaviorTreeFactory factory;
+  ASSERT_NO_THROW(factory.registerBehaviorTreeFromFile(path));
+
+  try
+  {
+    factory.registerBehaviorTreeFromFile(path);
+    FAIL() << "expected RuntimeError";
+  }
+  catch(const RuntimeError& e)
+  {
+    const std::string msg = e.what();
+    EXPECT_NE(msg.find(abs_path), std::string::npos) << msg;
+  }
+}
+
+TEST(DuplicateBehaviorTreeId, ClearAllowsReload)
+{
+  TempSubdir dir;
+  const auto path = dir.path / "clear.xml";
+  writeFile(path, minimalTreeXml("cleared"));
+
+  BehaviorTreeFactory factory;
+  ASSERT_NO_THROW(factory.registerBehaviorTreeFromFile(path));
+  factory.clearRegisteredBehaviorTrees();
+  ASSERT_NO_THROW(factory.registerBehaviorTreeFromFile(path));
+  ASSERT_NO_THROW(static_cast<void>(factory.createTree("cleared")));
+}
+
+TEST(DuplicateBehaviorTreeId, DifferentIdsCoexist)
+{
+  TempSubdir dir;
+  const auto path_a = dir.path / "tree_a.xml";
+  const auto path_b = dir.path / "tree_b.xml";
+  writeFile(path_a, minimalTreeXml("TreeA"));
+  writeFile(path_b, minimalTreeXml("TreeB"));
+
+  BehaviorTreeFactory factory;
+  ASSERT_NO_THROW(factory.registerBehaviorTreeFromFile(path_a));
+  ASSERT_NO_THROW(factory.registerBehaviorTreeFromFile(path_b));
+
+  const auto ids = factory.registeredBehaviorTrees();
+  ASSERT_EQ(ids.size(), 2);
+  ASSERT_NO_THROW(static_cast<void>(factory.createTree("TreeA")));
+  ASSERT_NO_THROW(static_cast<void>(factory.createTree("TreeB")));
+}
+
+TEST(DuplicateBehaviorTreeId, IncludeDuplicateThrows)
+{
+  TempSubdir dir;
+  const auto inner_path = dir.path / "inner.xml";
+  const auto outer_path = dir.path / "outer.xml";
+
+  writeFile(inner_path, minimalTreeXml("foo"));
+
+  const std::string outer_xml = R"(
+<root BTCPP_format="4">
+  <include path="inner.xml"/>
+  <BehaviorTree ID="foo">
+    <AlwaysSuccess/>
+  </BehaviorTree>
+</root>
+)";
+  writeFile(outer_path, outer_xml);
+
+  const std::string inner_abs = std::filesystem::absolute(inner_path).string();
+
+  BehaviorTreeFactory factory;
+  try
+  {
+    factory.registerBehaviorTreeFromFile(std::filesystem::absolute(outer_path));
+    FAIL() << "expected RuntimeError";
+  }
+  catch(const RuntimeError& e)
+  {
+    const std::string msg = e.what();
+    EXPECT_NE(msg.find(inner_abs), std::string::npos) << msg;
+  }
+}

tests/gtest_factory.cpp

@@ -400,6 +400,7 @@ TEST(BehaviorTreeReload, ReloadSameTree)
     ASSERT_EQ(NodeStatus::SUCCESS, tree.tickWhileRunning());
   }
 
+  factory.clearRegisteredBehaviorTrees();
   factory.registerBehaviorTreeFromText(xmlB);
   {
     auto tree = factory.createTree("MainTree");

tests/gtest_parallel.cpp

@@ -486,6 +486,8 @@ TEST(Parallel, ParallelAll)
     ASSERT_EQ(1, observer.getStatistics("third").success_count);
   }
 
+  factory.clearRegisteredBehaviorTrees();
+
   {
     const char* xml_text = R"(
 <root BTCPP_format="4">

tests/gtest_subtree.cpp

@@ -126,6 +126,8 @@ TEST(SubTree, BadRemapping)
   Tree tree_bad_in = factory.createTree("MainTree");
   EXPECT_ANY_THROW(tree_bad_in.tickWhileRunning());
 
+  factory.clearRegisteredBehaviorTrees();
+
   static const char* xml_text_bad_out = R"(
 <root BTCPP_format="4" >