fix(brain): restore agent_types/skill_types compat shims dropped in #542 - #621
Conversation
PR #542 deleted the brain_client.agent_types and brain_client.skill_types star-import shims, which every pre-#542 custom agent and skill in the field imports — after an OS update, all of them roster as broken with ModuleNotFoundError (all 21 custom agents on R7-27 were down). Restore both shims verbatim, export Agent/SkillRef/InputRef from the innate facade so new agents author against the stable namespace, and add regression tests: the legacy and facade import paths must load an agent onto the roster, and the shims must re-export the same class objects (registration and isinstance depend on identity).
Greptile SummaryRestores compatibility for agents and skills authored against the pre-refactor module paths while exposing agent authoring types through the stable
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
LegacyAgent["Legacy agent<br/>brain_client.agent_types"] --> AgentShim["agent_types shim"]
FacadeAgent["New agent<br/>innate.Agent"] --> AgentTypes["brain_client.agents.types"]
AgentShim --> AgentTypes
LegacySkill["Legacy skill<br/>brain_client.skill_types"] --> SkillShim["skill_types shim"]
SkillShim --> SkillTypes["brain_client.skills.types"]
AgentTypes --> AgentDiscovery["Agent discovery and roster"]
SkillTypes --> SkillDiscovery["Skill workspace discovery"]
Reviews (2): Last reviewed commit: "test(brain): legacy skill loads end-to-e..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def test_skill_types_shim_reexports_same_objects(): | ||
| for name in ( | ||
| "Skill", | ||
| "SkillResult", | ||
| "SkillOutput", | ||
| "SkillCancelled", | ||
| "RobotState", | ||
| "RobotStateType", | ||
| "Interface", | ||
| "InterfaceType", | ||
| ): | ||
| assert getattr(skill_types, name) is getattr(skills_types, name) |
There was a problem hiding this comment.
This test checks the skill shim only through direct imports, while legacy custom skills use the workspace discovery and registration path. Adding a dynamic-loading regression like the agent-side test would prevent loader-specific compatibility regressions from passing these identity assertions unnoticed.
Knowledge Base Used: brain_client: agent/skill runtime and ROS bridge
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good catch — added in 6f2bb4d: a legacy-authored skill (old brain_client.skill_types import, class-attribute Interface/RobotState declarations, tuple return) now loads end-to-end through import_workspace_packages + registered_workspace_skills, the same discovery functions _load_code_skills calls. The fixture clears Skill._registry so discovery sees only the synthetic workspace — other test modules register Skills (some deliberately broken), and discovery both rosters and prunes whatever the live registry holds.
Greptile flagged that the skill shim was only covered by direct-import identity checks while field skills load via discovery. Add a legacy-authored skill (old import path, Interface/RobotState declarations, tuple return) loaded through import_workspace_packages + registered_workspace_skills — the same functions the catalog calls. The fixture clears Skill._registry (other test modules register Skills, some deliberately broken, and discovery both rosters and prunes whatever the live registry holds).
2 participants
Problem
PR #542 deleted
brain_client/agent_types.pyandbrain_client/skill_types.py— the star-import compat shims for the pre-refactor module paths. Every custom agent and skill in the field authored before #542 imports those paths, so after an OS update they all roster as broken withModuleNotFoundError: No module named 'brain_client.agent_types'. On robot R7-27 that was all 21 custom agents.Fix
git show db3e33fe^). Star-import re-export means class identity is preserved, so__init_subclass__registration, the loader, and isinstance checks work with zero special-casing — the loader doesn't know the shim exists.Agent,SkillRef,InputReffrom theinnatefacade so new agents author against the stable public namespace (from innate import Agent) instead ofbrain_clientinternals — shims protect the past, the facade protects the future.agents/types.pyis import-light (no ROS at module level), so this adds no weight toimport innate.test_legacy_and_facade_import_paths_load— an agent authored against the old path and one againstinnateboth load onto the roster (the exact field regression).test_compat_shims.py— both shims re-export the same class objects as the new modules.Verification
ruff check+ruff format --check: clean on all touched files*.launch.pycollection errors on main are pre-existing)discover_agent_classes+build_agent_instancesover the real workspace → 0 import errors, 0 broken, 28 agents loaded (21 custom) — previously all 21 custom agents were broken.