Skip to content

WIP: Agent refactor#1211

Open
paul-nechifor wants to merge 5 commits intodevfrom
pauln-agent-refactor
Open

WIP: Agent refactor#1211
paul-nechifor wants to merge 5 commits intodevfrom
pauln-agent-refactor

Conversation

@paul-nechifor
Copy link
Contributor

No description provided.

@greptile-apps
Copy link

greptile-apps bot commented Feb 7, 2026

Greptile Overview

Greptile Summary

  • Replaces the prior dimos.agents implementation with a new dimos.agents3 Agent and skill3 discovery mechanism.
  • Removes the old skill protocol/coordinator plumbing (dimos.protocol.skill.*, dimos.core.skill_module) and updates blueprints/tests to match.
  • Updates CLI tooling/docs (removes skillspy, adds dotenv loading) and shifts system prompt import to agents3.
  • MCP integration is left in an inconsistent state: MCP bridge entrypoints remain, but MCPModule is effectively removed.

Confidence Score: 1/5

  • This PR is not safe to merge because MCP exports/blueprints are currently broken and the new skill/tool discovery path likely raises at runtime.
  • The changes delete the prior skill + MCP plumbing but leave dimos.protocol.mcp exporting MCPModule while the implementation is commented out, and unitree blueprints still reference it. Separately, get_skills3() builds schemas via langchain_core.tools.tool(attr).args_schema.model_json_schema(), which is not a stable contract and can raise during module discovery; the new Agent tool routing also appears to target RPC endpoints using Skill3Info.class_name rather than the actual RPC service name.
  • dimos/protocol/mcp/mcp.py, dimos/protocol/mcp/init.py, dimos/robot/unitree_webrtc/unitree_go2_blueprints.py, dimos/core/module.py, dimos/agents3/agent.py

Important Files Changed

Filename Overview
README.md Removes README mention of skillspy monitoring tool.
dimos/agents/init.py Removes all exports from dimos.agents package (now empty), which may break external imports expecting these symbols.
dimos/agents3/agent.py Introduces new threaded Agent implementation and dynamic tool creation from modules; contains RPC routing/schema concerns flagged in review comments.
dimos/core/module.py Removes SkillContainer base and adds get_skills3 discovery; schema generation approach likely breaks at runtime (flagged).
dimos/protocol/mcp/mcp.py Entire MCPModule implementation is commented out, but package still exports MCPModule; this breaks imports and MCP blueprints.
dimos/robot/cli/dimos.py Loads dotenv at startup and removes skillspy CLI subcommand.
dimos/robot/unitree_webrtc/unitree_go2_blueprints.py Switches to agents3 Agent blueprint and removes human_input; still references MCPModule blueprint which is currently broken.
pyproject.toml Removes skillspy console script entry.

Sequence Diagram

sequenceDiagram
    participant CLI as dimos CLI
    participant MC as ModuleCoordinator
    participant Agent as agents3.Agent
    participant Mod as Other Modules
    participant RPC as RPC (LCM)
    participant MCP as MCP Bridge (stdio<->tcp)

    CLI->>MC: deploy(Agent, modules...)
    CLI->>MC: start_all_modules()
    MC->>Mod: start()
    MC->>Agent: start()
    MC->>Agent: on_system_modules([RPCClient...])
    Agent->>RPC: get_skills3() (on each module)
    RPC-->>Agent: list[Skill3Info]
    Agent->>Agent: create_agent(tools from Skill3Info)

    Note over MCP: MCP expects a TCP server from MCPModule
    MCP->>RPC: connect(host:port)
    MCP-->>MCP: forwards JSON-RPC over TCP

    Agent->>Mod: tool call via RpcCall (remote_name/class_name)
    Mod-->>Agent: RPC response (or None if nowait)
    Agent-->>CLI: publish messages/idle state
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 374 to +377
result = tuple(self._bound_rpc_calls[m] for m in methods)
return result[0] if len(result) == 1 else result

@rpc
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tool schema generation bug

get_skills3() calls tool(attr).args_schema.model_json_schema() and then json.dumps(...). langchain_core.tools.tool() returns a Tool-like wrapper, and args_schema is not guaranteed to exist (or to be a Pydantic model with model_json_schema()), which will raise at runtime when enumerating skills. This needs to use a stable schema source (e.g., require __skill3__ functions to have an explicit Pydantic args_schema, or use StructuredTool.from_function(...).args_schema and then serialize that model/schema).

Comment on lines +137 to +140
def _get_tools_from_modules(
agent: Agent, modules: list[RPCClient], rpc: RPCSpec
) -> list[StructuredTool]:
skills = [skill for module in modules for skill in (module.get_skills3() or [])]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RpcCall target mismatch

_skill_to_tool() constructs RpcCall(..., name=skill.func_name, remote_name=skill.class_name, ...). remote_name is the module’s actor/service name used by RPCClient (typically module.actor_class.__name__), not an arbitrary class_name string returned by get_skills3(). If Skill3Info.class_name is the underlying Python class name (or differs from the deployed actor name), tool calls will be routed to a non-existent RPC endpoint. This should carry and use the actual RPC service name for the module instance that exposed the skill (or build the tool directly from the corresponding RPCClient instance rather than from a detached Skill3Info).

@greptile-apps
Copy link

greptile-apps bot commented Feb 7, 2026

Additional Comments (2)

dimos/protocol/mcp/mcp.py
MCPModule is removed

dimos.protocol.mcp.__init__ still exports MCPModule, but dimos/protocol/mcp/mcp.py now has the entire MCPModule implementation commented out. Importing from dimos.protocol.mcp import MCPModule will fail at import time, and any blueprint that references MCPModule.blueprint() will crash. Restore a real MCPModule implementation (or remove/replace the export and all call sites).


dimos/robot/unitree_webrtc/unitree_go2_blueprints.py
Broken MCP blueprint reference

This blueprint still wires in MCPModule.blueprint() but dimos.protocol.mcp.mcp.MCPModule no longer exists (the file is commented out). As-is, running unitree_go2_agentic_mcp will crash during import/blueprint creation. Either reintroduce MCPModule or remove/replace this blueprint dependency consistently.

@paul-nechifor paul-nechifor changed the title Agent refactor WIP: Agent refactor Feb 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant