fix(load_module): re-execute when cached __file__ differs from requested path

CI hang on `tests/test_cli/test_run.py::TestRunFailsWhenConnectionClassNotPresent::test_run`
diagnosed after every platform_checks matrix run on this branch hit the
120-min workflow timeout cancel.

Symptom: the test overwrites a connection's `connection.py` with a stub
that lacks `HTTPClientConnection`, runs the agent via the CLI, and
expects `ClickException("Connection class 'HTTPClientConnection' not
found")`. After commit `a0d759e7c` (drop __file__ equality from
cache-hit check), `load_module` returns the cached real connection from
`sys.modules` regardless of which file the caller pointed it at. The
class IS found (from the cache), the agent boots, enters its asyncio
event loop, and runs forever.

Root cause: the `a0d759e7c` relaxation was too broad. It addressed the
"Found many classes with name 'EchoBehaviour'" CI failure by always
reusing the cache, but in doing so it silently bypasses any caller that
points `load_module` at a different physical file — which is exactly
what test fixtures that stub source files on disk do.

Fix: restore the `__file__` equality guard. Cache-hit only when the
cached module's `__file__` resolves to the same path as the requested
`filepath`. When paths differ, fall through to spec-from-file-location
and re-execute against the file the caller pointed us at. The
block-import sentinel (`sys.modules[k] is None`) still raises
`ImportError` as before. On exec failure the half-built stub is
popped — a stale prior entry (different `__file__`) is discarded
rather than restored, since the caller was asking us to load a
different file.

Tests:
- `test_load_module_reuses_cached_module_for_same_dotted_path` renamed
  to `test_load_module_reuses_cached_module_for_same_file` and rewritten:
  the "different file at the same dotted path" branch now asserts the
  cache is bypassed and the new source is executed (Other defined,
  Marker not), instead of the old "still reuse the cached module"
  assertion.

Verified locally:
- `pytest tests/test_cli/test_run.py::test_run[None]
  tests/test_cli/test_run.py::TestRunFailsWhenConnectionClassNotPresent::test_run`
  passes in 1.4s (was hanging at 120s+).
- `pytest tests/test_skills/test_base.py
  tests/test_contracts/test_base.py tests/test_connections/test_base.py`:
  217 passed, 1 skipped, 1 error (Docker-related, unrelated).
- `pytest tests/test_cli/test_run.py tests/test_configurations/`:
  377 passed, 5 skipped.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: DIvyaNautiyal07

aea/helpers/base.py

@@ -127,31 +127,37 @@ def load_module(dotted_path: str, filepath: Path) -> types.ModuleType:
     :raises ValueError: if the filepath provided is not a module.  # noqa: DAR402
     :raises Exception: if the execution of the module raises exception.  # noqa: DAR402
     """
-    # If the dotted path is already in ``sys.modules``, reuse it instead
-    # of re-executing the file. Re-execution creates a duplicate copy of
-    # every class defined in the source, breaking class identity for
-    # callers that already hold a reference (e.g. via an earlier
-    # ``from ... import X``). ``__file__`` is intentionally NOT compared
-    # — in test fixtures the same logical package may be reached
-    # through different physical files (vendor copy in a tmpdir vs the
-    # source tree) but both register the same dotted path and must
-    # resolve to the same module object across the whole process. An
-    # explicit ``None`` entry is CPython's block-import sentinel: raise
+    # If the dotted path is already in ``sys.modules`` AND the cached
+    # module's ``__file__`` resolves to the same path we were asked to
+    # load, reuse it. Reuse preserves class identity for callers that
+    # already hold a reference (e.g. via an earlier ``from ... import
+    # X``). When the requested ``filepath`` differs from the cached
+    # ``__file__`` (a caller replaced the source on disk, or two distinct
+    # copies of the same package live at different paths), re-execute
+    # so the caller sees the file they pointed us at. An explicit
+    # ``None`` entry is CPython's block-import sentinel: raise
     # ``ImportError`` to match the standard import semantics rather
     # than silently overwriting it with a fresh exec.
     if dotted_path in sys.modules:
         existing = sys.modules[dotted_path]
         if existing is None:
             raise ImportError(f"import of {dotted_path!r} halted; None in sys.modules")
-        return existing
+        existing_file = getattr(existing, "__file__", None)
+        if existing_file is not None:
+            try:
+                same_file = Path(existing_file).resolve() == Path(filepath).resolve()
+            except OSError:
+                same_file = False
+            if same_file:
+                return existing
     spec = importlib.util.spec_from_file_location(dotted_path, str(filepath))
     module = importlib.util.module_from_spec(cast(ModuleSpec, spec))
     # Register before exec so a downstream `import` of the same dotted
-    # path during exec resolves to this module. We only reach this point
-    # when the key was absent (the cache-hit and block-import branches
-    # above covered the other two cases), so on exec failure we pop the
-    # half-built stub rather than restoring a prior entry that, by
-    # construction, did not exist.
+    # path during exec resolves to this module. On exec failure pop the
+    # half-built stub — if a stale entry (different ``__file__``) was
+    # present it is discarded rather than restored, since the caller
+    # was asking us to load a different file and we have no reason to
+    # leave the stale module visible.
     sys.modules[dotted_path] = module
     try:
         spec.loader.exec_module(module)  # type: ignore

tests/test_skills/test_base.py

@@ -511,17 +511,16 @@ def test_load_module_pops_sys_modules_on_exec_failure(tmp_path):
         sys.modules.pop(dotted_path, None)
 
 
-def test_load_module_reuses_cached_module_for_same_dotted_path(tmp_path):
-    """`load_module` reuses any module already cached at the dotted path.
-
-    Whether the second call points at the same physical file or a
-    different one (e.g. a vendor copy in a tmpdir vs the source tree
-    in test fixtures), `load_module` returns the cached object — never
-    re-executes. Re-execution would create duplicate class objects
-    for callers that already hold references to the original
-    definitions (e.g. via an earlier ``from ... import X``). Mirrors
-    Python's own import semantics: once registered, the cached module
-    is canonical for that dotted path.
+def test_load_module_reuses_cached_module_for_same_file(tmp_path):
+    """`load_module` reuses the cached module when the file matches.
+
+    Re-executing the same source would create duplicate class objects
+    and break class identity for callers that already hold a reference
+    (e.g. via an earlier ``from ... import X``). When the caller asks
+    for the same dotted path with a *different* physical file, the
+    cache is bypassed and the new file is executed — the caller has
+    explicitly pointed `load_module` at a different source and
+    expects to see it.
     """
     import sys
 
@@ -538,22 +537,21 @@ def test_load_module_reuses_cached_module_for_same_dotted_path(tmp_path):
         first = load_module(dotted_path, same_src)
         first_marker = first.Marker  # type: ignore[attr-defined]
 
-        # Same file → reuse.
+        # Same file → cache hit, identical module object.
         second = load_module(dotted_path, same_src)
         assert second is first, "load_module re-executed for the same file"
         assert second.Marker is first_marker  # type: ignore[attr-defined]
 
-        # Different file at the same dotted path → still reuse the
-        # cached module. ``Marker`` survives; ``Other`` is NOT defined
-        # on the returned module because the second source was never
-        # executed.
+        # Different file at the same dotted path → re-execute. The
+        # returned module reflects the new source: ``Other`` is
+        # defined, ``Marker`` is not.
         third = load_module(dotted_path, other_src)
-        assert third is first, (
-            "load_module re-executed when the cached dotted path was "
-            "called with a different physical file"
+        assert third is not first, (
+            "load_module returned the stale cached module instead of "
+            "executing the new source"
         )
-        assert hasattr(third, "Marker")
-        assert not hasattr(third, "Other")
+        assert hasattr(third, "Other")
+        assert not hasattr(third, "Marker")
     finally:
         sys.modules.pop(dotted_path, None)
 
@@ -1217,7 +1215,8 @@ class TestSkillLoadingWarningMessages(BaseAEATestCase):
     _TEST_BEHAVIOUR_CLASS_NAME = "TestBehaviour"
 
     _test_skill_module_path = "skill_module_for_testing.py"
-    _test_skill_module_content = dedent(f"""
+    _test_skill_module_content = dedent(
+        f"""
     from aea.skills.base import Behaviour, Handler
 
     class {_TEST_HANDLER_CLASS_NAME}(Handler):
@@ -1241,7 +1240,8 @@ def act(self):
             pass
         def teardown(self):
             pass
-    """)
+    """
+    )
 
     @classmethod
     def setup_class(cls):