Skip to content

aimdb-mcp tools open a new AimX connection per call, which breaks the serial session #451

Closed

Description

@lxsaah

Problem

Most aimdb-mcp tools open a fresh AimX connection on every call. Over serial this breaks the session that get_record and drain_record share. A UART never closes, so a board learns about a new client only from a hello, and a hello resets the board's session.

So, on a serial:// endpoint, each of these tool calls either:

  • fails, because aimdb-mcp's persistent client already has the tty open (tokio-serial opens it exclusively), or
  • if it does get through, sends a new hello, which resets the board's session and discards the drain cursor drain_record was reading.

#450 fixed this for get_record and drain_record only. They now share the persistent client from ConnectionPool::get_drain_client(), and that client is replaced only when its engine has stopped (AimxConnection::is_closed()).

Affected callers

All of these go through ConnectionPool::get_connection(), which always calls AimxConnection::connect:

Tool File
list_records, set_record tools/aimdb-mcp/src/tools/record.rs
get_instance_info tools/aimdb-mcp/src/tools/instance.rs
graph_nodes, graph_edges, graph_topo_order tools/aimdb-mcp/src/tools/graph.rs
get_buffer_metrics, reset_buffer_metrics tools/aimdb-mcp/src/tools/buffer_metrics.rs
get_stage_profiling, reset_stage_profiling tools/aimdb-mcp/src/tools/profiling.rs

Each also falls back to its own AimxConnection::connect when the pool is not initialized.

The records resource (tools/aimdb-mcp/src/resources/records.rs) also calls AimxConnection::connect directly. It only reads discovered Unix-socket instances today, so it is lower priority, but it should use the same client.

To reproduce

  1. Run a board with SerialServer and a ring record with remote access.
  2. Through aimdb-mcp, call drain_record on the ring to open its cursor.
  3. Publish some events on the board.
  4. Call list_records (or any tool in the table) on the same serial:// endpoint.
  5. Call drain_record again.

Expected: step 4 succeeds, and step 5 returns the events from step 3.
Actual: step 4 fails because the tty is busy. If it gets through instead, step 5 returns nothing, because the board's session and its cursor were reset.

Proposed fix

  • Route every tool and the records resource through the persistent per-endpoint client: get_drain_client(), take the per-client lock, one RPC per call.
  • Keep the same rules as fix: record.get answers at once and no longer drains; MCP get/drain share one session (#444) #450:
    • an error reply (not_found, unsupported, denied, even internal) keeps the session;
    • the client is replaced only when is_closed() reports that its engine has stopped.
  • Then remove get_connection() / invalidate_connection() and the per-tool fallbacks, so no path opens a second connection. Consider renaming get_drain_client(), since it would no longer be drain-specific.

Tests

Extend tools/aimdb-mcp/tests/serial_session.rs, which puts a SerialServer board behind a pty and counts the hellos it receives:

  • open a ring's drain cursor and publish events;
  • call each affected tool once;
  • drain again.

Every call should succeed, the drain should return the queued events, and the board should receive exactly one hello.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions