Fix: UnsetBlackboard follows SubTree remapping

[scopenhagenPickNik]

[written by AI]

Refs PickNikRobotics/moveit_pro#23361

UnsetBlackboard silently leaves a parent value set when called inside a SubTree through an explicit remap, _autoremap, or an @ root key. Reads and writes resolve those keys to the owning blackboard, but Blackboard::unset() only searched local storage.

Make unset() follow the same resolution order as getEntry(): root prefix, local entry, explicit remap, then automatic remapping for non-private keys. Release the local storage lock before recursing to a parent. Local entries retain precedence, _ keys remain private unless explicitly remapped, and missing keys remain a no-op. Update the method and built-in node descriptions to document the behavior.

This intentionally changes removal through remapped and root keys. Callers must be rebuilt because unset() is inline. MoveIt Pro adoption requires a ros-jazzy-behaviortree-cpp-picknik package release and its Dockerfile dependency bump; this PR contributes the underlying library fix.

Validation

• Added 11 regression tests covering explicit and automatic remapping, nested SubTrees, root access, private keys, local precedence, missing keys, an expired parent, sibling visibility, and rewriting after removal. Seven fail against the original implementation and all pass with the fix.
• Built the fork in an isolated Ubuntu 24.04 development container and ran ctest --test-dir build --output-on-failure --timeout 60 --parallel 1: 578 tests passed. Parallel CTest runs exposed existing logger fixtures sharing /tmp/bt_logger_test; the serial run passes without fixture changes.
• Compiled the actual MoveIt Pro IsBlackboardVariableSet source and its updated test suite against the patched fork: 16 tests passed. Its three remapping regressions fail before the fix and pass afterwards.
• pre-commit run -a passed. ./run_clang_tidy.sh build could not run because clangd-21 is unavailable; the pre-commit wrapper skips that check when the tool is missing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Team
• Run ID: 700c8a28-b157-4928-b48f-3333d184d9ee

📥 Commits

Reviewing files that changed from the base of the PR and between c3bc675 and b45d9ed.

📒 Files selected for processing (3)

• include/behaviortree_cpp/actions/unset_blackboard_node.h
• include/behaviortree_cpp/blackboard.h
• tests/gtest_blackboard.cpp

Included review availability: This review used your included allowance. 5 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour.

---

📝 Summary

Summary by CodeRabbit

• New Features
  • Unsetting a blackboard entry now follows subtree remapping, automatic remapping, and the @ root-key prefix, allowing the intended entry to be removed through those routes.
• Bug Fixes
  • Missing entries are now a no-op, and an unset can continue through remapping when no local entry exists.
  • Keys beginning with _ are not automatically remapped, and local entries take precedence over parent entries.

Walkthrough

Blackboard::unset now removes entries according to root-prefix and subtree remapping rules. New tests cover direct blackboard calls and UnsetBlackboard tree-node use.

Changes

Blackboard unset routing

Layer / File(s)	Summary
Unset routing behavior include/behaviortree_cpp/blackboard.h, include/behaviortree_cpp/actions/unset_blackboard_node.h	unset routes @-prefixed keys to the root. It removes a local entry when present; otherwise, it forwards through explicit remapping or eligible automatic remapping. Comments describe the routing rules and missing-entry behavior.
Unset routing tests tests/gtest_blackboard.cpp	Tests cover local and missing entries, explicit and automatic remapping, private keys, nested mappings, root-prefixed keys, expired parents, and XML use of UnsetBlackboard.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to b45d9

No actionable merge-blocking issue is established; the change is mergeable after normal checks.

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore (reviewers only)

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Human Review Check	❌ Error	This PR changes the public behavior of BT::Blackboard::unset() in the public header. The method now follows root-key, explicit remapping, and automatic remapping rules instead of only removing local…	This PR requires review by a requested human reviewer. After review, a non-author requested reviewer should override this pre-merge check.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description is complete and relevant. It explains the bug, resolution order, behavioral and compatibility impact, documentation changes, regression tests, validation results, and the …
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Human Review Check

Explanation

This PR changes the public behavior of BT::Blackboard::unset() in the public header. The method now follows root-key, explicit remapping, and automatic remapping rules instead of only removing local entries. The public contract is also documented as changed, and the method is inline, so callers must be rebuilt. This matches the custom check condition for public API changes. No MoveIt Pro package files are changed.

• Fix all pre-merge checks with AI

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Pre-merge checks failed. Please resolve the failing checks before merging.