Feature: advanced xml validation

[vivoeiva]

I was looking into different BT.CPP xml file examples (at least they look so at the first glance) and found out that some of them are actually not completely valid. Moreover, trying to open them in Groot2 brings it to an endless loop. Sadly there is no obvious way to validate these files, so I'd like to introduce one.
In general, the following functionality is added:

1. creation of 'generic' xsd file to check against built-ins without any user added nodes.
2. creation of a Schematron file checking for advanced constraints like TreeNodesModel for Groot2.
3. tools for generating both.
4. generated schemas in the repository, these potentially can be automatically updated
5. and a python script to check any given xml file against them

It also seems that not all xml files in the repository (these that should be valid) pass the validation. I would address it in a different PR if needed.

NB: LLM (Claude) was used to improve and contribute to the code where my knowledge is limited (Schematron in particular). All LLM contributions have been reviewed by me.

[vivoeiva]

I'd like to give also some context on results:

./tools/validate_xml.py fuzzing/corpus/bt_corpus/corpus1.xml
fuzzing/corpus/bt_corpus/corpus1.xml validates OK

./tools/validate_xml.py fuzzing/corpus/bt_corpus/corpus2.xml
fuzzing/corpus/bt_corpus/corpus2.xml:5: element Timeout: Schemas validity error : Element 'Timeout', attribute 'timeout_ms': The attribute 'timeout_ms' is not allowed.
fuzzing/corpus/bt_corpus/corpus2.xml:5: element Timeout: Schemas validity error : Element 'Timeout': The attribute 'msec' is required but missing.
fuzzing/corpus/bt_corpus/corpus2.xml fails to validate

./tools/validate_xml.py fuzzing/corpus/bt_corpus/corpus3.xml
fuzzing/corpus/bt_corpus/corpus3.xml:4: element SetBlackboard: Schemas validity error : Element 'SetBlackboard', attribute 'key': The attribute 'key' is not allowed.
fuzzing/corpus/bt_corpus/corpus3.xml:4: element SetBlackboard: Schemas validity error : Element 'SetBlackboard': The attribute 'output_key' is required but missing.
fuzzing/corpus/bt_corpus/corpus3.xml fails to validate
fuzzing/corpus/bt_corpus/corpus3.xml: /root/BehaviorTree/Sequence/SubTree: SubTree ID='SubTree' is not defined in this file.
        (If the definition lives in an included file, add an <include>.)

./tools/validate_xml.py fuzzing/corpus/bt_corpus/corpus4.xml
fuzzing/corpus/bt_corpus/corpus4.xml:5: element ForceSuccess: Schemas validity error : Element 'ForceSuccess': Missing child element(s). Expected is one of ( {*}*, * ).
fuzzing/corpus/bt_corpus/corpus4.xml:6: element ForceFailure: Schemas validity error : Element 'ForceFailure': Missing child element(s). Expected is one of ( {*}*, * ).
fuzzing/corpus/bt_corpus/corpus4.xml fails to validate

[facontidavide]

Thanks for this contribution — the layering is the right approach (XSD for structure, Schematron for cross-file-reference rules that XSD can't express), and the tests are a good start. I reviewed the diff in detail; a few things should be addressed before merging, listed roughly by severity.

Should be fixed before merge

1. The checked-in tools/generated/bt4.xsd is already stale relative to the code in this PR.
writeTreeXSD() now emits inoutPortType and accepts <inout_port> inside treeNodesModelNodeType, but the committed bt4.xsd contains neither. Since WriteTreeToXML() emits <inout_port> for bidirectional ports, any tree saved by the library with an inout port fails validation against the shipped default schema used by validate_xml.py. Please regenerate. More generally, drift appearing within the PR itself suggests we need a CI job that regenerates tools/generated/* and fails on diff — otherwise these files will rot.

2. CMakeLists.txt: editing src/btcpp4_schematron.sch does not trigger reconfiguration.
configure_file() only tracks its input template (schematron_template.h.in); a file consumed via file(READ) is not added to CMAKE_CONFIGURE_DEPENDS, so the comment above that block is incorrect and edits to the .sch will silently keep building with the stale embedded string. Fix:

set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS
    "${CMAKE_CURRENT_SOURCE_DIR}/src/btcpp4_schematron.sch")

3. The writeTreeXSD signature change is an ABI break.
Adding the defaulted bool generic parameter changes the mangled symbol, so existing binaries linked against libbehaviortree_cpp.so calling the old writeTreeXSD(const BehaviorTreeFactory&) would fail to load. Please keep the old signature and add an overload instead:

[[nodiscard]] std::string writeTreeXSD(const BehaviorTreeFactory& factory);
[[nodiscard]] std::string writeTreeXSD(const BehaviorTreeFactory& factory, bool generic);

4. Schematron Rule B false-positives on built-in nodes written in explicit notation.
Rule A excludes builtins via the ##BUILTIN_PIPE## list, but Rule B (<Action ID="..."/>, <Control ID="..."/>, …) requires a TreeNodesModel entry for every ID. <Control ID="Sequence"> or <Action ID="AlwaysSuccess"/> is valid BT.CPP XML but would be reported as missing a model entry. Rule B needs the same guard, e.g. not(contains('##BUILTIN_PIPE##', concat('|', @ID, '|'))).

Nice to have

5. Placeholder substitution in writeTreeSchematron() is single-shot and fails silently.
Only the first ##BUILTIN_PIPE## occurrence is replaced, and if the placeholder is absent (stale generated header, template edit) the function returns the template unchanged — producing a Schematron that treats every builtin as a custom node, with no error signal. A replace-all loop plus throwing when zero replacements occur would make this fail loudly. This is also the root cause of the awkward \#\# escaping in the template comment; a more distinctive placeholder would avoid that.

6. tools/generated/bt4.sch carries template-only artifacts.
The generated copy still contains the \#\#BUILTIN_PIPE\#\# escape backslashes and the "expanded at runtime" comment, which is misleading in an already-expanded file. Worth stripping or rewording during generation. (Same CI-regeneration remedy as point 1 keeps the two .sch copies from drifting.)

7. validate_xml.py with --no-xsd --no-sch prints "validates OK" and exits 0 while validating nothing. A guard that errors when both are disabled would prevent a misleading green result in scripts/CI.

Non-issues I checked

• The or /root/include relaxation disabling the SubTree check when any <include> exists is a reasonable XPath 1.0 tradeoff, and it's documented.
• The new port-attribute sorting (ordered_ports) changes attribute order in the strict XSD but makes the output deterministic — good change.
• factory.builtinNodes() returns a std::set, so the pipe list is deterministic.

Overall this is a valuable feature and I'd like to land it. Points 1–4 are the blockers; 5–7 are improvements.