From 05cf51572b8051526711262048978e1e22fd9947 Mon Sep 17 00:00:00 2001
From: mustafab0 <mustafabhadsorawala@gmail.com>
Date: Sat, 8 Aug 2026 15:49:44 -0700
Subject: [PATCH] fix(core): match module refs by IS-A so a subclass provider
 resolves
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A module ref declared as a concrete class (`coordinator: ControlCoordinator`)
was matched with `cls is spec`. A deployment that swaps in a subclass — to add
per-instance ports, say — therefore satisfied nothing: _resolve_single_ref fell
through to returning the spec class, get_instance() found no deployed instance
of it, and the ref was set to None. Nothing raised at wiring time; the failure
surfaced later as an AttributeError on the first call through the ref.

Match with issubclass instead, which is what a declared dependency means.

This is not hypothetical: the per-instance command-stream change in this branch
moves eight manipulator deployments onto ControlCoordinator subclasses, which
silently unbound ManipulationModule._control_coordinator (trajectory execution,
gripper get/set) in ten blueprints and ArmCommandModule.coordinator (E-STOP) in
the two hosted ones. Verified across every blueprint in all_blueprints: twelve
non-optional refs went unresolved before this fix and none after, with no ref
becoming ambiguous.

Only ControlCoordinator and GO2Connection are referenced by concrete class
anywhere in the tree; every other ref goes through a *Spec, which already
matched structurally and so accepted subclasses. GO2Connection carried the
same latent trap for whoever subclassed it next.
---
 dimos/core/coordination/module_coordinator.py |  7 +++++-
 .../coordination/test_module_coordinator.py   | 25 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/dimos/core/coordination/module_coordinator.py b/dimos/core/coordination/module_coordinator.py
index 24b7a70ce2..332f17c093 100644
--- a/dimos/core/coordination/module_coordinator.py
+++ b/dimos/core/coordination/module_coordinator.py
@@ -942,7 +942,12 @@ def _resolve_single_ref(
     is_class_ref = is_module_type(spec)
 
     def satisfies(cls: type) -> bool:
-        return cls is spec if is_class_ref else spec_structural_compliance(cls, spec)
+        # A subclass IS-A the declared provider, so a deployment that swaps in a
+        # subclass (extra ports, per-instance I/O) still satisfies the ref. Exact
+        # identity would resolve to None here, silently.
+        if is_class_ref:
+            return isinstance(cls, type) and issubclass(cls, spec)
+        return spec_structural_compliance(cls, spec)
 
     def module_of(candidate: Any) -> type[ModuleBase]:
         return candidate.module if isinstance(candidate, BlueprintAtom) else candidate
diff --git a/dimos/core/coordination/test_module_coordinator.py b/dimos/core/coordination/test_module_coordinator.py
index 2e0481fbcb..05266310b8 100644
--- a/dimos/core/coordination/test_module_coordinator.py
+++ b/dimos/core/coordination/test_module_coordinator.py
@@ -209,6 +209,11 @@ def start(self) -> None:
     def stop(self) -> None: ...
 
 
+# How a deployment specializes a provider: extra I/O, same RPC surface.
+class Calculator1WithPort(Calculator1):
+    extra_in: In[Image]
+
+
 def _build_without_rerun(blueprint: Blueprint) -> ModuleCoordinator:
     """Build with a parsed viewer override so tests never spawn Rerun."""
     parsed = BlueprintConfigParser(blueprint).parse(
@@ -496,6 +501,26 @@ def test_module_ref_direct() -> None:
         coordinator.stop()
 
 
+def test_module_ref_direct_accepts_a_subclass_provider() -> None:
+    # A deployment may swap in a subclass to add per-instance ports. Matching
+    # the ref by exact class identity would leave Mod1.calc set to None, with
+    # no error at wiring time and an AttributeError at first use.
+    coordinator = _build_without_rerun(
+        autoconnect(
+            Calculator1WithPort.blueprint(),
+            Mod1.blueprint(),
+        )
+    )
+
+    try:
+        mod1 = coordinator.get_instance(Mod1)
+        assert mod1 is not None
+        assert mod1.calc is not None
+        assert mod1.calc.compute1(2, 3) == 5
+    finally:
+        coordinator.stop()
+
+
 def test_module_ref_spec() -> None:
     coordinator = _build_without_rerun(
         autoconnect(