Port PickNik's fork onto upstream BehaviorTree.CPP 4.9.0 - #29
Draft
dv-picknik wants to merge 162 commits into
Draft
Port PickNik's fork onto upstream BehaviorTree.CPP 4.9.0#29dv-picknik wants to merge 162 commits into
dv-picknik wants to merge 162 commits into
Conversation
Signed-off-by: redvinaa <redvinaa@gmail.com>
Signed-off-by: redvinaa <redvinaa@gmail.com>
Co-authored-by: ahuo <ahuo2865189826@gmail.com>
…#1009) Co-authored-by: ahuo <ahuo2865189826@gmail.com>
…e#1007) * fix: use dynamically growing error buffer in ParseScript * style: format code * fix: use dynamically growing error buffer in ValidateScript --------- Co-authored-by: ahuo <ahuo2865189826@gmail.com>
…lite3.h won't be found (BehaviorTree#1002) Co-authored-by: alejandro.suarez@omron.com <alejandro.suarez@omron.com>
* Refactor VerifyXML to clarify logic - Reduces duplication in VerifyXML by handling the ID check for built-in node types up front so they can then be definitively looked up in the registered nodes. - Enhances error messaging in VerifyXML by using *either* the node name *or* the ID, depending on which is appropriate, instead of leaving users guessing "which Decorator is wrong" - Fixes custom Action and Condition nodes using shorthand syntax not being properly verified - Fixes `<Control ID="ReactiveSequence"/>` not being verified with the same logic as `<ReactiveSequence/>` - Fixes `<Action ID="MyAction"/>` not triggering a behavior lookup when `<MyAction/>` would. * fix tests that were failing due to bad assumptions
* Support using minitrace from conan
* Support using tinyxml2 from conan
* Add support for using minicoro from conan
* Add support for using flatbuffers from conan
* Create separate targets for each 3rdparty lib not yet supported by conan so we can avoid exposing the whole 3rdparty folder on target_include_directories
Since this can create some confusion around which headers are actually being included -- the ones from that folder or the ones from conan?
Also fixes the include dirs by using ${CMAKE_CURRENT_SOURCE_DIR} instead of "."
* Fix builds
For whatever reason including zmq.hpp before zmq_addon.hpp (which does include zmq.hpp internally) breaks builds
* Do not include the whole 3rdparty folder, only link in what we need
* Use the regular lexy target
* Do not expose the whole 3rdparty folder as a include_directory
* Add options to opt-out of vendored libraries
* This was shared across both code paths, conan_build.cmake and ament_build.cmake
So it is better to keep this on a single place
* Keep all the find_package calls on the toplevel CMakeLists
* SQLite3 is actually a dependency of cpp-sqlite
* Fix include dirs of the vendored minicoro and flatbuffers
* Define libzmq cmake target on FindZeroMQ to match the conan package
* Improve message. This code path doesn't really mean we're using conan, it just means we're not using ament.
* Use the python version of conanfile.py so we can set the CMake options needed to opt out of vendored dependencies
* Address pre-commit complains
* Use conanfile.py across the board
* Do not look for ZeroMQ directly as it is a dependency of cppzmq
Also only look for cppzmq if BTCPP_GROOT_INTERFACE
* Keep a single copy of zmq.hpp
This header is part of the cppzmq library so it lives on 3rdparty/cppzmq. But for whatever reason there was
another version of this header here. Furthermore it was a differnt version of the library.
* Leave a FIXME for posterity
This target was silently being skiped, not it is explicit
* Leave comment for posterity
* Remove empty line
* Remove unneeded line
* Remove uneeded line
* Remove empty line
* Revert unintentional changes
* Use cmake_layout to support multiconfig
* Update toolchain path on cicd
* Emtpy commit to re-trigger CI
* Use cppzmq from conan
* Use cmake presets on conan builds
* It looks like in windows the preset is called conan-default
* It looks like the preset is only called default for config?
* Fix tests path in windows
* Use lexy from conan
* Force cppstd to 17, conan profile detect uses 14
* Try to fix windows builds
* Remove wildcards cmake options, it has been removed on master
* Update changes after cpp-sqlite removal
this modern approach registers many individual tests instead of a single monolitic test so if one fails the rest continue running which allows the developer to flag multiple failing tests on a single run It also speeds up testing since tests run in parallel
- Add run_clang_tidy_hook.sh for pre-commit integration - Skips if clangd-21 not installed - Skips if compile_commands.json not found - Runs only on modified files - Fails on errors - Fix clang-tidy warnings in exception tracking code: - Use anonymous namespace instead of static for tick_stack_ - Add deleted move operations to TickStackGuard Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…avior Demonstrates that regular Sequence does NOT re-evaluate conditions while a child action is RUNNING, whereas ReactiveSequence DOES. This is expected behavior - users should use ReactiveSequence when they need conditions to be re-evaluated every tick. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…-sequence-vs-reactive Add test for Issue BehaviorTree#819: Sequence vs ReactiveSequence behavior
When a new commit is pushed to a PR, any running jobs for that PR will be automatically canceled, reducing CI queue time and noise. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…Tree#1109) * Fix clang-tidy warnings across tests, examples, and samples - Fix uninitialized std::array counters across test files - Add default member initializers in test node classes - Add anonymous namespaces for file-local functions - Add NOLINT comments for intentional patterns - Fix implicit bool conversions - Fix self-assignment in copy assignment operators - Add Rule of Five compliance to test fixtures - Exclude optional-dependency files from clang-tidy (gmock, zmq, flatbuffers) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix heap corruption caused by thread_local tick stack The thread_local std::vector used for exception backtraces caused heap corruption on Windows DLLs and when running all tests in a single process. Simplified NodeExecutionError to store only the failing node info instead of a full tick backtrace, removing the thread_local entirely. Also disabled Groot2Publisher tests (to be addressed separately) and fixed Groot2Publisher destructor cleanup. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…sions. (BehaviorTree#1110) Co-authored-by: Davide Faconti <davide.faconti@gmail.com>
…aviorTree#1107) Enable passing shared_ptr<Derived> to ports expecting shared_ptr<Base> without breaking ABI compatibility. Users register inheritance relationships at runtime via BehaviorTreeFactory::registerPolymorphicCast, and the system handles upcasting/downcasting transparently. Key components: - PolymorphicCastRegistry: thread-safe registry for inheritance relationships with transitive cast support (e.g. Sphynx->Cat->Animal) - BehaviorTreeFactory owns the registry, shares it with Blackboards - Blackboard::tryCastWithPolymorphicFallback: public Expected-returning API used by both Blackboard::get and TreeNode::getInputStamped - XML parser validates port compatibility including polymorphic types - Downcasts allowed at parse-time, checked at runtime via dynamic_pointer_cast Design decisions: - Registry uses shared_mutex for read-heavy workload - Error propagation via nonstd::expected<T, std::string> - No circular includes: safe_any.hpp includes registry directly - All polymorphic tests consolidated in gtest_polymorphic_ports.cpp Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…#1112) EntryUpdatedDecorator::halt() was missing the call to DecoratorNode::halt(), which meant the child node was never halted when the decorator was halted. Fixes BehaviorTree#1111 Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
New control node that executes a try-child and, on failure, runs a catch-child as recovery. Supports configurable catch_on_halt behavior, async children, and re-entry after RUNNING states. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add null check for subtree_id when <SubTree> in <TreeNodesModel> is missing the ID attribute — now throws descriptive RuntimeError instead of crashing with UB (null pointer as map key) - Fix variable name shadowing: rename inner-loop 'name' to 'port_name' to avoid shadowing the outer structured binding Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix 6 data races in Blackboard, verified with ThreadSanitizer (clang-21): - set() new-entry path: wrote value/sequence_id/stamp without entry_mutex after createEntryImpl — add scoped_lock on entry_mutex - set() existing-entry path: held raw reference after unlocking storage_mutex_, risking use-after-free if concurrent unset() erases the entry — copy shared_ptr before unlocking - cloneInto(): read/wrote entry members without entry_mutex — add scoped_lock on src and dst entry mutexes - ImportBlackboardFromJSON(): wrote entry->value without any lock — add scoped_lock on entry_mutex - debugMessage(): iterated storage_ without storage_mutex_ — add unique_lock - getKeys(): iterated storage_ without storage_mutex_ — add unique_lock Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Switch storage_mutex_ from std::mutex to std::shared_mutex so that concurrent readers (getEntry, getKeys, debugMessage, set initial lookup, cloneInto snapshot) no longer block each other. Write operations (createEntryImpl, unset, clear, cloneInto insert/erase) still take exclusive locks. Also remove the dead entry_mutex_ recursive_mutex and its deprecated entryMutex() accessor — no callers exist in the codebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix Blackboard thread-safety bugs and XML null pointer dereference
…ut<T>` to convert `vector<BT::Any>` type to `vector<T>` * Support vector<Any> -> vector<typename T::value_type> conversion Don't check port type alignment for vector<Any> * Convert vector to vector<Any> before placing on the blackboard Also update checks to allow mismatch when a port was declared as a vector<T> and we have an input port that takes it in as a vector<Any> * Update include/behaviortree_cpp/blackboard.h Co-authored-by: Nathan Brooks <nbbrooks@gmail.com> * Fix formatting with pre-commit * Add unit test passing a vector through ports --------- Co-authored-by: Nathan Brooks <nbbrooks@gmail.com> (cherry picked from commit 663cfa3)
This aligns with how custom types are represented by the JsonExporter. * Update both toJson & fromJson functions in the JsonExporter * Add & update tests Co-authored by: David Sobek <david.sobek@picknik.ai> (cherry picked from commit c06e058)
Signed-off-by: Paul Gesel <paul.gesel@picknik.ai> (cherry picked from commit 79e09ed)
Port-integration fix, no upstream counterpart. Upstream 4.9.0's LoopNode (issue BehaviorTree#969) accepts SharedQueue<T>, std::vector<T>, or a string in its 'queue' port. The fork's setOutput<std::vector<T>>() stores vectors as std::vector<BT::Any>, so none of those casts match and LoopNode throws "port 'queue' must contain either SharedQueue<T>, std::vector<T>, or a string". This is a genuine fork-vs-upstream collision, not a merge artifact: upstream's test PortTest.LoopNodeAcceptsVector_Issue969 fails without this. Unwrap std::vector<BT::Any> element-by-element back to T, erroring with the element's own cast failure rather than the generic message when a member does not fit the loop type. Refs PickNikRobotics/moveit_pro#17640 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX
Use type_ID instead of element->Name() so the error prints the actual behavior name (e.g. GetEpickObjectDetectionStatus) rather than the generic XML tag (e.g. Action). Use element->GetLineNum() instead of att->GetLineNum() for a correct line number. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> (cherry picked from commit 4d87c12)
…numbers (cherry picked from commit 959730c)
(cherry picked from commit 9ab097e)
Ported from fork commits 2a0f48f + 5596408. Upstream 4.9.0 has not adopted an equivalent: it still re-parses _autoremap inside recursivelyCreateSubtree, and still assigns every subtree remapping to config.input_ports regardless of the direction declared in <TreeNodesModel>. Deliberate divergence from the original fork commits: NodeConfig::auto_remapped is APPENDED after post_conditions rather than inserted after output_ports. The original placement shifted other_attributes/manifest/uid/path/pre_conditions/ post_conditions by 8 bytes, and because TreeNode::getInput<T>() is header-inlined into callers and dereferences config().manifest, binaries built against stock headers read that pointer out of the middle of other_attributes and crash. That was ABI break #1 in moveit_pro#20928. Appending restores every offset: member stock 4.9.0 original fork this commit other_attributes 144 152 144 manifest 200 208 200 uid 208 216 208 path 216 224 216 pre_conditions 248 256 248 post_conditions 296 304 296 Verified with offsetof against a stock 4.9.0 checkout, not by inspection. Dropped from the original commits: the tests/CMakeLists.txt hunk (targeted a catkin branch and a test-target name 4.9.0 no longer has) and the gmock dependency it existed to pull in -- the two EXPECT_THAT matchers are rewritten as plain gtest, so the fork adds no build-time dep upstream lacks. Refs PickNikRobotics/moveit_pro#17640, PickNikRobotics/moveit_pro#20928 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX
Squash of fork commits be74f60 + 19bfaf2. They must land together: without 19bfaf2's empty() guard, `--value.end()` on an empty vector is UB, so be74f60 alone is a latent crash rather than a separable feature. Upstream 4.9.0 has no toStr overload for std::vector<T>, but it does stringify vectors via the toJsonString fallback plus the BehaviorTree#965 JSON vector converters, so this is a change of output format, not a new capability, for the four registered element types: vector<int>{1,2,3,4} stock: json:[1,2,3,4] fork: 1;2;3;4 vector<string>{} stock: json:[] fork: "" vector<float> stock: throws LogicError fork: 1.000000;2.000000 Both round-trip -- 4.9.0 taught convertFromString<vector<T>> to strip a "json:" prefix and it still accepts ';'-delimited input. The fork form additionally covers element types with a toStr<T> specialization but no JSON converter, which stock 4.9.0 throws on. Dropped from the original commits: the throw_unspecialized_error lambda. The compiler cannot see that a lambda call always throws, so it warns "control reaches end of non-void function" at both call sites and the fall-through is UB. Upstream's inline throw is kept instead. Refs PickNikRobotics/moveit_pro#17640 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX
Ported from fork commits 8972919 + b8d70d6, rebuilt against 4.9.0's build system rather than cherry-picked -- upstream moved BTCPP_*_DESTINATION into the top-level CMakeLists.txt and rewrote tools/CMakeLists.txt, so the original hunks no longer apply. Keeps the coexistence invariants from docs_nonpublic/design/behaviortree-cpp-fork-coexistence.md: scoped ament package / library / include root, versioned soname (now .so.4.9, was .so.4.7), disjointly-named bin/ tools, and the BTCPP_PICKNIK_FORK compile-time sentinel. New in 4.9.0 and therefore NOT covered by the original rename commits: upstream added tools/bt_nodes_model.cpp, installing /opt/ros/*/bin/bt4_nodes_model. Stock ros-jazzy-behaviortree-cpp 4.9.0 installs that exact path, so shipping it unrenamed is a hard dpkg "trying to overwrite" failure -- and a path-exclude does not help, because dpkg's ownership-conflict check runs before the exclude filter. Renamed to bt4_picknik_nodes_model. Note git cherry-pick auto-merges this block below the conflict region, so it survives a careless resolution silently; moveit_pro's Dockerfile assertion would have caught it only after a full image build. Dropped as obsolete on 4.9.0: - the LEXY_ENABLE_INSTALL block -- 4.9.0 deleted 3rdparty/lexy outright, replacing it with a hand-written tokenizer/parser. Every remaining vendored tree (tinyxml2, minitrace, minicoro, flatbuffers, cppzmq, doxygen-awesome-css) is STATIC or INTERFACE with no install() rule, so none leaks into the export set and the collision class does not recur. - the tests/CMakeLists.txt test-target rename -- the test binary is never installed, so it is not collision surface. Refs PickNikRobotics/moveit_pro#17640, PickNikRobotics/moveit_pro#20928 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX
Ported from fork commit f4c4ce8 (test only). The refactor that shipped with it (cc77236) and its test fixups (ceadf1a) are dropped: upstream 4.9.0 already contains that refactor essentially verbatim -- same is_builtin precondition block, same lookup_name dispatch, same reordered BehaviorTree/SubTree branches -- and goes further with validateModelName, a TryCatch arity check and a kMaxNestingDepth guard. The fork's version is a strict subset, so cherry-picking it would regress upstream. The assertions are rewritten with std::string::find instead of gmock's HasSubstr, so the fork does not add a gmock build dependency upstream lacks. That is also why ceadf1a's package.xml and tests/CMakeLists.txt hunks are dropped -- they existed only to pull gmock in for these two matchers. Note both fixtures trip on an unregistered node (TriggerServer, IsDoorLocked) rather than on a structural rule, so this guards recursive line-number reporting, not decorator arity. Refs PickNikRobotics/moveit_pro#17640 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX
|
Important Review skippedToo many files! This PR contains 388 files, which is 288 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (388)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Upstream 4.9.0 added validateModelName, which rejects ' ' in <BehaviorTree ID>,
<SubTree ID>, and custom node type names. MoveIt Pro names every Objective and
SubTree in human-readable form, so this rejects essentially the entire product.
Swept moveit_pro and moveit_pro_example_ws for names that 4.9.0's
findForbiddenChar rejects:
C++ port names 1226 port-creation sites, 1310 kPortID* constants: 0
XML tree / subtree IDs 1139
XML TreeNodesModel node IDs 809
----------
total 1948 across 334 files, 276 distinct
1926 of those are in shipped config rather than test fixtures -- core Objectives
like "Close Gripper", "Move to Pose", "Wait for Trajectory Approval if User
Available" -- and customer configs in the field use the same convention.
Renaming them is not an option: Objective IDs are referenced by saved configs.
Every one of the 1948 is the space character. No name in either repo uses any
other forbidden character, so relaxing exactly ' ' clears all of them and
nothing else. Verified by re-running the sweep under the relaxed rule: 0.
'<' '>' '&' '"' '\'' (break XML serialization) and '/' '\\' ':' '*' '?' '|' '.'
are all still rejected, and a new test pins that. Upstream already permits
spaces in instance names for the same human-readability reason, so this narrows
the model/instance gap rather than inventing a rule.
ABI-safe: validateModelName lives in src/xml_parsing.cpp, not a header, so this
changes no struct layout and no header-inlined body.
The two upstream tests asserting spaces are rejected now assert the fork's
behavior instead, renamed to say so.
Refs PickNikRobotics/moveit_pro#17640
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX
dv-picknik
marked this pull request as draft
August 5, 2026 20:22
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
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.
Advances the fork from the 4.7.2 line to upstream 4.9.0, which is what nav2's Jazzy debs are built against. This is the fix #20928 concluded was necessary — the full commit-by-commit breakdown is on moveit_pro#17640.
11 commits on top of upstream tag
4.9.0, one per feature. 511/511 tests pass, zero compiler warnings.Why a replay and not a merge
The fork's history contains duplicate commits — PR #15 rebased
mainonto the upstream mirror and then merged the pre-rebase line back in, so most features exist twice. A merge produces one unreviewable blob. Replaying commit-by-commit also answers the question that actually matters — is this patch still needed on 4.9.0? — and for 9 of the 22 the answer was no.What this closes
All three ABI breaks from #20928:
NodeConfig::auto_remappedinserted mid-struct, shiftingmanifestand everything after by 8 bytesJsonExporter::from_json_array_converters_SimpleStringmove ctor gainednoexceptin 4.9.0, flippinglinb::anybetween heap and inline storageBreak #3 is why the layout-patch approach on
fix/20928-btcpp-abiwas abandoned: sizes, offsets and mangled names are all identical, so no layout diff can see it. That branch is now retired.ABI break #1, verified by measurement
offsetofagainst a stock 4.9.0 checkout, same toolchain:other_attributesmanifestuidpathpre_conditionspost_conditionsgit cherry-pickauto-mergestree_node.hwith no conflict and silently reinserts the member mid-struct. Resolving conflicts carefully is not enough to avoid reshipping the bug. A one-lineoffsetofassertion in the suite would have caught #20928 at the commit that introduced it — worth adding separately.Packaging collision found
4.9.0 adds
tools/bt_nodes_model.cpp→/opt/ros/*/bin/bt4_nodes_model. Stockros-jazzy-behaviortree-cpp4.9.0 installs that exact path, so shipping it unrenamed is a harddpkg"trying to overwrite" failure — and per the coexistence design's own pitfall section, apath-excludedoes not help, because dpkg's ownership check runs before the exclude filter. Renamed tobt4_picknik_nodes_model. This block also auto-merges silently, below the conflict region.Conversely the
LEXY_ENABLE_INSTALLguard is now obsolete — 4.9.0 deleted3rdparty/lexyoutright, and every remaining vendored tree is STATIC/INTERFACE with noinstall()rule.Testing
pixi run build && pixi run test— 511/511, zero warningslibbehaviortree_cpp_picknik.so.4.9.0with versioned soname.so.4.9behaviortree_cpp_picknik,bt4_picknik_plugin_manifest,bt4_picknik_nodes_modelNodeConfigoffsets measured against a stock 4.9.0 worktreeOne upstream test,
PortTest.LoopNodeAcceptsVector_Issue969, failed mid-port: the fork'ssetOutputstores vectors asvector<Any>, which upstream's newLoopNodedoesn't recognise. A real fork-vs-upstream collision, not a merge artifact — fixed in its own commit.Decisions that want a reviewer
{"__type":"double","value":3.14}; stock emits bare3.14. Kept the fork'stoJson(the Pro UI consumes it), madefromJsonaccept both. The tagged path is gated on__typenaming a primitive rather than on avaluefield merely existing — otherwise a registered custom type with its ownvaluemember, or the diagnostic blob the non-registered-type commit emits, imports silently as astd::string.1;2;3;4, stock givesjson:[1,2,3,4]; empty vector""vsjson:[]. Both round-trip. The fork form additionally covers types with atoStr<T>specialization but no JSON converter (vector<float>), which stock throws on. Easiest thing to drop if nothing downstream needs;.'5' .. 3returns an emptyAny— the widenedare_numbersguard steals theconcatbranch. Live in the fork today; the port neither introduces nor fixes it. One-token fix (&& op != concat); left alone to keep the port faithful.Blocking risk before this ships
4.9.0 rejects
. / \ : < > & " ' * ? |in port names and BehaviorTree/SubTree IDs (findForbiddenChar+validateModelName). A port namedrequest.namethat works today throws at tree-load time. Needs a sweep ofmoveit_proandmoveit_pro_example_wsObjective XML — a literal-only grep is not enough, since most Behaviors declare ports viakPortID*constants.Also outstanding, none blocking review of this branch:
input_ports/output_ports;bt_flatbuffer_helper.hiterates onlyinput_ports, so Groot1 serialization omits subtree OUTPUT ports.{=}expansion now applies to every node type, not just subtrees — a deliberate widening..so.4.7→.so.4.9:apt_build_farmneeds agit_shabump withrevisionreset to 1,Dockerfilepin →4.9.0-1noble, and every Pro binary rebuilt.package.xmladdslibsqlite3-dev,libzmq3-dev,tinyxml2,tinyxml2_vendor.Refs PickNikRobotics/moveit_pro#17640, PickNikRobotics/moveit_pro#20928
🤖 Generated with Claude Code
https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX