Add skill lanes for MCP tool serialization

[RitwijParmar]

Problem

Addresses #2204. DimOS skills currently do not expose whether a tool can run concurrently with another tool. That is risky for robot/world-mutating skills such as motion or manipulation: two MCP calls can be dispatched at the same time even when they should share an execution lane.

Closes DIM-XXX

Solution

• Add a parameterized @skill(lane="motion") form while preserving the existing bare @skill decorator.
• Carry the lane through SkillInfo discovery so module skill metadata knows which lane a skill belongs to.
• Include lane metadata in MCP tools/list responses under tool _meta.
• Serialize concurrent MCP tools/call requests that share the same non-empty lane, while leaving unlaned or differently-laned skills concurrent.

How to Test

• /tmp/dimos/.venv/bin/python -m ruff check dimos/agents/annotation.py dimos/core/module.py dimos/agents/mcp/mcp_server.py dimos/agents/test_skill_result.py dimos/agents/mcp/test_mcp_server.py
• /tmp/dimos/.venv/bin/python -m pytest dimos/agents/test_skill_result.py dimos/agents/mcp/test_mcp_server.py

Note: I used Python 3.12 through uv. A full uv run sync initially hit local pyaudio/PortAudio headers on this Mac, so I provisioned a minimal env for the focused files above.

Contributor License Agreement

• I have read and approved the CLA.

[greptile-apps]

Greptile Summary

This PR adds optional execution-lane support to the @skill decorator so that MCP tools/call requests sharing the same named lane are serialized, preventing concurrent mutation of robot state. Lane metadata is also surfaced in tools/list responses under _meta.lane.

• annotation.py gains a parameterized @skill(lane=\"motion\") form via typed overloads, with the lane stored in __skill_lane__ on the wrapped function.
• SkillInfo carries the new lane field, and mcp_server.py acquires a per-lane asyncio.Lock (lazily stored in the global app.state.lane_locks) before dispatching laned calls to the thread-pool executor.
• The test for same-lane serialization works today but shares the global app.state.lane_locks dict; since asyncio.Lock in Python 3.10+ binds to the first event loop that uses it, a second test with the same lane name in a fresh asyncio.run() would raise RuntimeError.

Confidence Score: 4/5

Safe to merge for production use; lane-serialization logic is correct in the single-event-loop uvicorn environment, but the global lock dict creates a test-isolation trap.

The production serialization behavior is sound — one event loop, locks never cross loop boundaries. The only real concern is app.state.lane_locks being a module-global not injectable through handle_request, which means loop-bound asyncio.Lock objects created in one asyncio.run() test session will silently persist and raise RuntimeError if a second test exercises the same lane in a fresh session. Current tests avoid this only because each lane name appears in exactly one test.

dimos/agents/mcp/mcp_server.py — the lane_locks global state and its interaction with handle_request parameter design.

Important Files Changed

Filename	Overview
dimos/agents/annotation.py	Refactors @skill into a two-form decorator (bare and parameterized); adds __skill_lane__ attribute and empty-string guard. Overloads are correctly typed.
dimos/agents/mcp/mcp_server.py	Adds lane_locks global state and per-lane asyncio.Lock serialization. Lock dict is module-level and not injectable through handle_request, creating a test-isolation gap where loop-bound locks persist across asyncio.run() calls.
dimos/core/module.py	Adds `lane: str
dimos/agents/mcp/test_mcp_server.py	Adds two new tests: lane metadata in tools/list and same-lane serialization. Tests share the global app.state.lane_locks, meaning a second test with the same lane name in a separate asyncio.run() would raise RuntimeError.
dimos/agents/test_skill_result.py	Adds unit tests for both decorator forms verifying __skill__ and __skill_lane__ attributes. Correct and self-contained.

Sequence Diagram

sequenceDiagram
    participant C as MCP Client
    participant H as handle_request
    participant TC as _handle_tools_call
    participant LL as lane_locks (global)
    participant EX as ThreadPoolExecutor

    C->>H: "tools/call {name: drive}"
    H->>TC: req_id, params, skills, rpc_calls, app.state.lane_locks
    TC->>TC: lane lookup from skills list
    alt lane is None
        TC->>EX: run_in_executor(rpc_call)
        EX-->>TC: result
    else lane is motion
        TC->>LL: setdefault(motion, asyncio.Lock())
        LL-->>TC: lock
        TC->>TC: async with lock acquire
        TC->>EX: run_in_executor(rpc_call)
        note over EX: Serialized second caller waits at lock
        EX-->>TC: result
        TC->>TC: release lock
    end
    TC-->>H: JSON-RPC result
    H-->>C: response

Loading

_{Reviews (1): Last reviewed commit: "Add skill lanes for MCP tool serializati..." | Re-trigger Greptile}

[greptile-apps]

Loop-bound asyncio.Lock in shared global dict

app.state.lane_locks is a module-level dict that persists across the process lifetime. In Python 3.10+, asyncio.Lock inherits from _LoopBoundMixin, which binds the lock to the first event loop that calls acquire() on it — subsequent calls from a different event loop raise RuntimeError: bound to a different event loop. Each asyncio.run() invocation (as used in tests) creates its own event loop, so after test_mcp_module_serializes_same_lane_calls completes, the "motion" lock remains in the dict bound to the now-closed loop. Any future test that also exercises the "motion" lane in a fresh asyncio.run() will immediately fail. The root cause is that lane_locks is not threaded through handle_request the way skills and rpc_calls are. Adding it as an optional parameter (defaulting to app.state.lane_locks) would let tests pass their own empty dict.

[greptile-apps]

The O(n) linear scan for the lane duplicates work already done when the rpc_calls dict was built. A {s.func_name: s.lane for s in skills}.get(name) dict comprehension or a pre-built lane map alongside rpc_calls would make this O(1).

Suggested change

	lane = next((s.lane for s in skills if s.func_name == name), None)
	lane = {s.func_name: s.lane for s in skills}.get(name)

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!