feat: add physicalai-lerobot-plugin - #265
Conversation
physicalai-lerobot-plugin
Dependency ReviewThe following issues were found:
Vulnerabilitiesuv.lock
License Issuespackages/physicalai-lerobot-plugin/pyproject.toml
OpenSSF ScorecardScorecard details
Scanned Files
|
|
|
||
| for _importer, module_name, is_package in pkgutil.walk_packages(package.__path__, prefix=f"{package_name}."): | ||
| if "config" in module_name and not is_package and _is_allowed_dynamic_import(module_name): | ||
| importlib.import_module(module_name) # nosemgrep: python.lang.security.audit.non-literal-import |
|
|
||
| for _importer, modname, is_pkg in pkgutil.walk_packages(lerobot.robots.__path__, prefix="lerobot.robots."): | ||
| if "config" in modname and not is_pkg and _is_allowed_dynamic_import(modname): | ||
| importlib.import_module(modname) # nosemgrep: python.lang.security.audit.non-literal-import |
| prefix="lerobot.teleoperators.", | ||
| ): | ||
| if "config" in modname and not is_pkg and _is_allowed_dynamic_import(modname): | ||
| importlib.import_module(modname) # nosemgrep: python.lang.security.audit.non-literal-import |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical runtime and discovery defects, along with packaging and dependency problems, block approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds physicalai-lerobot-plugin to Physical AI with LeRobot adapters, Studio catalog discovery, runtime examples, URDF assets, tests, and packaging metadata.
Changes:
- Adds robot and teleoperator adapters with lifecycle and configuration handling.
- Adds dynamic Studio registration, discovery, probing, and bundled URDF resources.
- Adds documentation, tests, release metadata, and locked LeRobot/Torch/CUDA dependencies.
File summaries
| File | Summary and finalized review notes |
|---|---|
uv.lock |
Locks the plugin and LeRobot dependency graph, including transitive Torch/CUDA packages. |
packages/physicalai-lerobot-plugin/urdf/lerobot/urdf/lerobot.urdf |
Adds the bundled LeRobot URDF. |
packages/physicalai-lerobot-plugin/urdf/lerobot/meshes/.gitkeep |
Preserves the mesh resource directory. |
packages/physicalai-lerobot-plugin/tests/test_studio_catalog.py |
Tests catalog discovery, payloads, registration, builders, and assets. |
packages/physicalai-lerobot-plugin/tests/test_lerobot_adapter.py |
Tests adapter behavior and lifecycle handling. |
packages/physicalai-lerobot-plugin/tests/conftest.py |
Provides test fixtures and SDK stubs. |
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/studio_catalog.py |
Registers dynamic catalog definitions. Findings: line 606 critical (3 votes)—serial-only probing marks network robots offline; line 648 moderate (3 votes) and line 666 moderate (1 vote)—payload model construction can abort later registrations; lines 93 and 467 nit (1 vote each)—add rationale for dynamic-import suppressions; line 110 moderate (2 votes)—native discovery exceeds the allowlist; line 128 critical (1 vote)—namespace prefixes are not a sufficient trust boundary. |
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py |
Implements robot and teleoperator adapters. Findings: lines 136 moderate (3 votes)—device lock IDs are not canonical; lines 443 and 742 moderate (3 votes)—images are not wrapped as Frame objects; lines 364 and 484 critical (2 votes)—failed robot startup can leak partial state; lines 658 and 786 critical (2 votes)—failed teleoperator startup can leak partial state; line 159 nit (1 vote)—document the import suppression; line 162 moderate (2 votes)—runtime discovery remains insufficiently filtered. |
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/constants.py |
Defines plugin constants. |
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/_urdf.py |
Resolves bundled URDF resources. Finding: line 16 critical (3 votes)—the wheel path resolution climbs above site-packages and should probe the adjacent urdf directory first. |
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/__init__.py |
Initializes and exposes the plugin package. |
packages/physicalai-lerobot-plugin/README.md |
Documents installation and usage. Findings: line 3 nit (1 vote)—repository attribution and links still point to the old project; line 167 nit (1 vote)—the referenced development guide does not exist. |
packages/physicalai-lerobot-plugin/pyproject.toml |
Defines packaging, dependencies, entry points, tests, and assets. Findings: line 15 moderate (3 votes)—mandatory LeRobot dependencies pull a large CUDA/GPU stack; line 23 moderate (1 vote)—the tests extra lacks pytest-anyio; line 50 moderate (1 vote)—the package license is not included in the source distribution. |
packages/physicalai-lerobot-plugin/examples/runtime/teleop.yaml |
Adds a teleoperation recipe. Finding: line 30 critical (3 votes)—the leader should use LeRobotTeleoperatorAdapter, not LeRobotAdapter. |
packages/physicalai-lerobot-plugin/CHANGELOG.md |
Adds package release history through version 0.3.0. |
.github/release-please-config.json |
Configures release automation for the package. |
.github/.release-please-manifest.json |
Registers the package release version. Finding: line 6 moderate (1 vote)—the manifest starts at 0.2.3 while the changelog is at 0.3.0. |
Review details
Suppressed comments (12)
.github/.release-please-manifest.json:6
- The release manifest initializes this package at
0.2.3, while the checked-in changelog already has0.3.0as its latest release. Release Please will therefore compute from a stale version and can attempt to recreate or mis-sequence the existing release; set the manifest to the latest version represented by the migrated changelog.
"packages/physicalai-lerobot-plugin": "0.2.3",
packages/physicalai-lerobot-plugin/README.md:167
- This link targets
docs/creating-a-studio-plugin.md, but that file does not exist in this repository, so the development guide in the new package README is broken. Link to an existing guide or add the referenced document before publishing the package.
See [`docs/creating-a-studio-plugin.md`](../../docs/creating-a-studio-plugin.md) for the full plugin development guide.
packages/physicalai-lerobot-plugin/README.md:3
- The package was moved into this repository, but the README still identifies it as part of the original
MarkRedeman/physicalai-pluginsmonorepo (and the nearby screenshot links still point there). This contradicts the migration described by the PR and leaves users directed to the old project; update the repository attribution and links.
Bridges [LeRobot](https://github.com/huggingface/lerobot) robot and teleoperator configs into [PhysicalAI](https://github.com/openvinotoolkit/physicalai), the Python library and runtime for robot control, transport, and CLI workflows. It registers with [Physical AI Studio](https://github.com/open-edge-platform/physical-ai-studio), the application that discovers catalog plugins and provides robot setup, teleoperation, and workflow experiences. Part of the [physicalai-plugins](https://github.com/MarkRedeman/physicalai-plugins) monorepo.
packages/physicalai-lerobot-plugin/pyproject.toml:50
- There is no
packages/physicalai-lerobot-plugin/LICENSEin this change, so the sdist include entry is relative to a nonexistent file and the published source distribution omits the repository license. Use the root-licenseforce-includepattern used by the bimanual plugin instead.
"LICENSE",
packages/physicalai-lerobot-plugin/pyproject.toml:23
- The added suite uses
@pytest.mark.anyiointests/test_studio_catalog.py:358, but the package'stestsextra installs onlypytest. A standalone test install therefore lacks the async test plugin; includepytest-anyioin this extra.
tests = ["pytest"]
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:488
- After a successful connect followed by
disconnect(),_robotremains non-None, so this check permitssend_action()to call the LeRobot device even thoughis_connected()is false. Require the wrapped robot to be connected here; otherwise a runtime can command a closed/disconnected device.
robot = self._robot
if robot is None:
msg = "Robot is not connected. Call connect() first."
raise ConnectionError(msg)
robot.send_action(action_dict)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:789
- The teleoperator adapter has the same disconnected-device hole:
disconnect()leaves_teleoperatorset, and this check does not testis_connected()before forwarding feedback. A follower-role teleoperator can therefore receive actions after disconnect; require an active connection before callingsend_feedback().
teleop = self._teleoperator
if teleop is None:
msg = "Teleoperator is not connected. Call connect() first."
raise ConnectionError(msg)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:743
- The teleoperator path repeats the same invalid cast: a NumPy image array is stored as a
Framewithout the required metadata. If a teleoperator emits an image-valued action,RobotObservation.imagesis structurally invalid; construct realFrameinstances with timestamps and sequences instead.
if isinstance(value, np.ndarray) and value.ndim >= _DIM_THRESHOLD_IMAGE:
images[key] = cast("Frame", value)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:159
- This dynamic-import suppression has no adjacent explanation of why the imported name is safe. Document that
package_nameis exact-root allowlisted andmodule_nameis restricted by_is_allowed_dynamic_import()before keeping the# nosemgrep(rule 1).
importlib.import_module(module_name) # nosemgrep: python.lang.security.audit.non-literal-import
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/studio_catalog.py:666
- The leader loop repeats the same pre-registration failure:
_make_payload_model()runs outside thetryblock inregister_physicalai_studio_plugin(). An incompatible third-party teleoperator schema will therefore stop all subsequent catalog entries instead of being skipped. Move this model construction under per-definition error handling.
payload_cls = _make_payload_model(config_cls)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/studio_catalog.py:93
- This
# nosemgrepsuppresses the dynamic-import finding without the required rationale. Add an adjacent comment explaining thatmodnameis enumerated from the allowlistedlerobot.robotstree and checked by_is_allowed_dynamic_import()before importing it (rule 1).
importlib.import_module(modname) # nosemgrep: python.lang.security.audit.non-literal-import
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/studio_catalog.py:467
- This second
# nosemgrepsuppression also lacks a safety justification. Explain inline that the module name is discovered only under the allowlistedlerobot.teleoperatorspackage and passes_is_allowed_dynamic_import()before retaining the suppression (rule 1).
importlib.import_module(modname) # nosemgrep: python.lang.security.audit.non-literal-import
- Files reviewed: 15/17 changed files
- Comments generated: 12
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| class_path: physicalai.runtime.TeleopSource | ||
| init_args: | ||
| leader: | ||
| class_path: physicalai_lerobot_plugin.lerobot_adapter.LeRobotAdapter |
| def get_urdf_path() -> Path: | ||
| traversal = ir.files("physicalai_lerobot_plugin") | ||
| with ir.as_file(traversal) as p: | ||
| return p.parent.parent.joinpath("urdf") |
| try: | ||
| self._connect_robot() | ||
| except Exception as e: | ||
| msg = f"Failed to connect LeRobot {self._robot}: {e}" | ||
| raise ConnectionError(msg) from e |
| try: | ||
| self._connect_teleoperator() | ||
| except Exception as e: | ||
| msg = f"Failed to connect LeRobot teleoperator {self._teleoperator}: {e}" | ||
| raise ConnectionError(msg) from e |
| _ALLOWED_DYNAMIC_IMPORT_PREFIXES: tuple[str, ...] = ( | ||
| "lerobot.", | ||
| "lerobot_robot_", | ||
| "lerobot_teleoperator_", | ||
| ) |
| def _device_ids(config_type: str, config_kwargs: dict[str, Any]) -> tuple[str, ...]: | ||
| """Return stable serial-device identities without touching hardware.""" | ||
| ports = sorted(set(_collect_device_ports(config_kwargs))) | ||
| return tuple(f"lerobot:{config_type}:{port}" for port in ports) |
| def _register_third_party_plugins() -> None: | ||
| from lerobot.utils.import_utils import register_third_party_plugins # noqa: PLC0415 | ||
|
|
||
| register_third_party_plugins() |
| if isinstance(value, np.ndarray) and value.ndim >= _DIM_THRESHOLD_IMAGE: | ||
| images[key] = cast("Frame", value) |
| from lerobot.utils.import_utils import register_third_party_plugins | ||
|
|
||
| register_third_party_plugins() |
| continue | ||
|
|
||
| display_name = f"LeRobot {type_str}" | ||
| payload_cls = _make_payload_model(config_cls) |
There was a problem hiding this comment.
🟡 Changes recommended
The adapter has unsupported action layouts, invalid image handling, packaging and example failures, and unresolved dependency and plugin-import risks.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (11)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:610
- This produces an empty joint/action layout for registered teleoperators whose actions do not use
.poskeys. For example, LeRobot 0.6.1 registers keyboard/gamepad teleoperators withdelta_x/delta_y/delta_zor velocity keys;get_observation()then returns a zero-length vector, making these catalog entries unusable. Use the teleoperator'saction_featuresas the authoritative layout or skip incompatible types.
pos_keys = sorted(k for k in action if k.endswith(_POSITION_KEY_SUFFIX))
self._joint_order = [k[: -len(_POSITION_KEY_SUFFIX)] for k in pos_keys]
self._num_joints = len(self._joint_order)
self._action_position_keys = list(pos_keys)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:444
- Casting the camera array does not create a
Frame;imagesstill contains a rawnp.ndarray. Runtime policy input later readsframe.data, and robot verification also requirestimestampandsequence, so any LeRobot config with a camera fails when embedded images are consumed. Wrap each image in a realFrameand maintain a per-source sequence counter.
if isinstance(value, np.ndarray) and value.ndim >= _DIM_THRESHOLD_IMAGE:
images[key] = cast("Frame", value)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:139
- Including
config_typein the lock identity means two adapters configured for the same serial device under different LeRobot aliases receive differentdevice_ids, sophysicalai.robot.transportcan open the same hardware concurrently. Derive serial identities withdevice_id_from_serial_port()and classify non-serial endpoints separately so identity depends on the physical endpoint, not the selected config type.
def _device_ids(config_type: str, config_kwargs: dict[str, Any]) -> tuple[str, ...]:
"""Return stable serial-device identities without touching hardware."""
ports = sorted(set(_collect_device_ports(config_kwargs)))
return tuple(f"lerobot:{config_type}:{port}" for port in ports)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:368
- If the initial observation fails after
robot.connect()succeeds, this rethrows while leaving the hardware connected. A retry then returns early fromconnect()becauseis_connected()is true, but joint discovery is still incomplete, leaving the adapter unusable and the port open. Disconnect before rethrowing so failed initialization is atomic.
try:
self._connect_robot()
except Exception as e:
msg = f"Failed to connect LeRobot {self._robot}: {e}"
raise ConnectionError(msg) from e
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:488
- This guard checks only whether an object was constructed, not whether it remains connected. After
disconnect(),_robotis retained and joint order is retained, so this path still invokessend_action()on a disconnected device instead of raising the documentedConnectionError. Checkrobot.is_connectedhere as well.
robot = self._robot
if robot is None:
msg = "Robot is not connected. Call connect() first."
raise ConnectionError(msg)
robot.send_action(action_dict)
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:662
- As with the robot adapter, a failure while reading the initial action can occur after the teleoperator connects. The exception leaves it connected, and subsequent
connect()calls no-op without discovering the action layout. Disconnect before wrapping and rethrowing the initialization error.
try:
self._connect_teleoperator()
except Exception as e:
msg = f"Failed to connect LeRobot teleoperator {self._teleoperator}: {e}"
raise ConnectionError(msg) from e
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/lerobot_adapter.py:790
- This checks only whether a teleoperator object exists. Because
disconnect()retains_teleoperatorand the discovered keys, follower-role callers can still reachsend_feedback()after disconnection. Includenot teleop.is_connectedin the guard so commands are never forwarded to a disconnected device.
teleop = self._teleoperator
if teleop is None:
msg = "Teleoperator is not connected. Call connect() first."
raise ConnectionError(msg)
teleop.send_feedback(feedback_dict)
packages/physicalai-lerobot-plugin/examples/runtime/teleop.yaml:30
so101_leaderis aTeleoperatorConfig, but this class resolves names only throughRobotConfig; running the documented example therefore fails with a missing config choice before connecting. InstantiateLeRobotTeleoperatorAdapterfor the leader.
class_path: physicalai_lerobot_plugin.lerobot_adapter.LeRobotAdapter
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/_urdf.py:19
- The wheel force-includes
urdfat the site-packages root, one level above the Python package, but this always returns two levels above. It works in the editablesrc/layout tested here and returns a nonexistent path after wheel installation. Probe bothp.parent / "urdf"and the editable-layout location, as the other bundled plugins do.
def get_urdf_path() -> Path:
traversal = ir.files("physicalai_lerobot_plugin")
with ir.as_file(traversal) as p:
return p.parent.parent.joinpath("urdf")
packages/physicalai-lerobot-plugin/pyproject.toml:15
- Because this package is matched by the root workspace wildcard and workspace members are synced by default, making
lerobotmandatory pulls Torch and the Linux CUDA stack into the normal repositoryuv sync(the lock now contains several gigabytes of NVIDIA wheels). Keep the plugin out of the workspace's default members or otherwise make LeRobot installation opt-in, while retaining it as a required dependency when this plugin itself is installed.
"lerobot>=0.6.0",
packages/physicalai-lerobot-plugin/src/physicalai_lerobot_plugin/studio_catalog.py:113
- This automatically imports every installed package accepted by LeRobot's name-prefix discovery. The local prefix predicate does not establish publisher trust, so an installed typosquatted
lerobot_robot_*orlerobot_teleoperator_*distribution executes code during Studio startup. Require explicit administrator opt-in/allowlisting for third-party modules, or clearly make trusted installation of extensions the security boundary rather than treating the prefixes as a security restriction.
from lerobot.utils.import_utils import register_third_party_plugins
register_third_party_plugins()
_LEROBOT_THIRD_PARTY_PLUGINS_IMPORTED = True
- Files reviewed: 15/17 changed files
- Comments generated: 3
- Review effort level: Balanced
| pos_keys = sorted(k for k in obs if k.endswith(_POSITION_KEY_SUFFIX)) | ||
| if not pos_keys: | ||
| pos_keys = sorted(k for k in obs if "position" in k.lower() or k.endswith("pos")) | ||
| self._joint_order = [_strip_position_suffix(k) for k in pos_keys] | ||
| self._num_joints = len(self._joint_order) | ||
| self._obs_position_keys = list(pos_keys) | ||
| self._act_position_keys = list(pos_keys) |
| "packages/physicalai-studio-plugin": "0.1.0", | ||
| "packages/physicalai-bimanual-so101-plugin": "0.2.3", | ||
| "packages/physicalai-rebot-b601-plugin": "0.4.3", | ||
| "packages/physicalai-lerobot-plugin": "0.2.3", |
| @@ -0,0 +1,167 @@ | |||
| # PhysicalAI LeRobot Plugin | |||
|
|
|||
| Bridges [LeRobot](https://github.com/huggingface/lerobot) robot and teleoperator configs into [PhysicalAI](https://github.com/openvinotoolkit/physicalai), the Python library and runtime for robot control, transport, and CLI workflows. It registers with [Physical AI Studio](https://github.com/open-edge-platform/physical-ai-studio), the application that discovers catalog plugins and provides robot setup, teleoperation, and workflow experiences. Part of the [physicalai-plugins](https://github.com/MarkRedeman/physicalai-plugins) monorepo. | |||
This PR moves the lerobot plugin from my repo (https://github.com/MarkRedeman/physicalai-plugins/tree/main/packages/physicalai-lerobot-plugin) to the physicalai repo.
One security aspect to note:
lerobot_robot_*,lerobot_teleoperator_*, other imports are rejectedOpens:
lerobot_robot_namespaceuv syncnow also pulls in cuda dependencies due to lerobot's dependency ontorch,torchvision, is there a good way to prevent this?