Fix #2320: narrow gateway/*.py edits via bare-name AST resolver

Makefile

@@ -277,9 +277,9 @@ lint-custom: sync-venv-if-uv
 ## per-branch Last-Known-Good (LKG) commit (or base branch when no
 ## LKG sidecar exists), and runs pytest on only that subset.  Any
 ## sign of static-analysis fog (conftest / Makefile / pyproject /
-## uv.lock / workflow / shared/tests / non-.py / gateway/*.py /
-## dynamic-import / unresolvable-baseline / LKG-not-ancestor) widens
-## to the full suite with an explicit trigger string on stderr.  See
+## uv.lock / workflow / shared/tests / non-.py / dynamic-import /
+## unresolvable-baseline / LKG-not-ancestor) widens to the full
+## suite with an explicit trigger string on stderr.  See
 ## docs/guides/testing.md and scripts/select_tests.py for the full
 ## design.
 ##

docs/guides/testing.md

@@ -103,9 +103,12 @@ by the `make test` recipe. The algorithm is:
      `from action_guards import …`) to fully-qualified grimp module
      ids, covering the test and production files that import via
      short names rather than fully-qualified package paths. Applies
-     to `shared.*`, `orchestrator.*`, and `sandbox.*`; `gateway.*`
-     is excluded — its importlib test-loader pattern is handled by
-     the `gateway/*.py` widening trigger (§7).
+     to `shared.*`, `orchestrator.*`, `sandbox.*`, AND `gateway.*` —
+     gateway tests reach production via `gateway/tests/conftest.py`'s
+     `importlib.spec_from_file_location` loader (which makes every
+     gateway production module importable by bare name), so the AST
+     resolver bridges those edges the same way it does for the
+     sys.path-injected packages. See §7 for the full rationale.
 6. **Map modules → test files.** Intersect the downstream set with
    the pre-collected set of every `test_*.py` / `*_test.py` file
    in the graph. The selector emits the resulting set of test file
@@ -183,17 +186,16 @@ suite, with the explicit trigger string written to stderr (e.g.
 | **`.github/workflows/test.yml` change** | The CI definition itself — running narrow risks misrepresenting CI's posture. |
 | **`shared/tests/**` change** | `shared/tests/conftest.py` is a universally-consumed cross-package fixture; v1 widens on any path under `shared/tests/` to avoid an allowlist that has not yet been audited. May narrow in a follow-up. |
 | **Any non-`.py` change** | Schemas, fixture data, scripts, YAML, Markdown — none are reachable via the import graph. Conservative v1 default; an allowlist of known-safe paths can be added later. |
-| **`gateway/*.py` change** (production files directly under `gateway/`, NOT `gateway/tests/`) | **Known static-analysis blind spot.** `gateway/tests/conftest.py`'s `_load_module_with_replaced_imports` loads production modules via `importlib.util.spec_from_file_location`, so gateway tests import production by **bare name** (`from policy import ...`, never `from gateway.policy import ...`). Grimp cannot see those edges. Without this trigger, `gateway/policy.py` edits would silently select zero tests. The explicit trigger string is `gateway source change (importlib test-loader)`. See §7 for the long form. |
 | **Source file missing from grimp graph** | At graph-construction time, the selector enumerates every non-test `.py` under `gateway/`, `shared/`, `orchestrator/`, `sandbox/` (excluding `__pycache__`, `.venv`, test directories) and asserts each path resolves to a node in `graph.modules`. Any miss (PACKAGES drift, encoding quirk, grimp cache bug) widens to the full suite with trigger `source file missing from graph: <path>`. |
 | **Dynamic-import-touched module** | During graph construction, the selector regex-scans each module for `importlib.{import_module,util,machinery}`, `__import__`, `SourceFileLoader`, and entry-point plugin patterns. If any changed module is in (or reverse-reachable from) that set, narrow analysis is unsafe. |
 | **Unresolvable changed path** | A changed path that cannot be mapped to an in-repo module (e.g. brand-new file not yet in grimp's graph, a `scripts/*.py` with no wheel binding). |
 | **Unresolvable baseline** | LKG missing AND `origin/<base>` missing or `merge-base` failing. |
 | **LKG not ancestor of HEAD** | The recorded LKG sha is not reachable from `HEAD` (force-push, reset, history rewrite). |
 
 When a fallback fires, the stderr line uses the **explicit trigger
-name** (e.g. `Makefile changed`, `gateway source change (importlib
-test-loader)`), not generic wording — the trigger reason is the
-single most useful diagnostic.
+name** (e.g. `Makefile changed`, `dynamic-import reachability`),
+not generic wording — the trigger reason is the single most useful
+diagnostic.
 
 ---
 
@@ -329,30 +331,39 @@ Static reverse import graphs are powerful, but they cannot see:
   do not have static import edges from their consumers. The
   dynamic-import scan picks up the consumer side; missed cases
   surface when CI runs `make test-all`.
-- **Bare-name imports for non-gateway packages.** `shared.*`,
-  `orchestrator.*`, and `sandbox.*` modules are almost universally
+- **Bare-name imports across packages.** `shared.*`, `orchestrator.*`,
+  `sandbox.*`, and `gateway.*` modules are almost universally
   imported by bare name throughout the codebase (e.g. `from
   egg_logging.signatures import …` rather than `from
-  shared.egg_logging.signatures import …`). Grimp registers modules
-  under fully-qualified names, so a plain grimp traversal misses
-  those edges. **Mitigation:** the bare-name AST resolver (step 5)
-  AST-scans every `.py` and maps bare-name targets back to
-  fully-qualified ids, making these edges visible to narrowing.
-- **Gateway's importlib test-loader.** `gateway/tests/conftest.py`
-  defines `_load_module_with_replaced_imports`, which uses
-  `importlib.util.spec_from_file_location` to load production
-  modules and then injects mocks via `sys.modules`. Every gateway
-  test imports production by **bare name** (`from policy import
-  ...`), so grimp sees zero edges from `gateway/tests/test_*.py`
-  to `gateway/policy.py`. Without mitigation, a `gateway/policy.py`
-  edit would resolve an empty test set. **Mitigation:** any change
-  to a file matching `gateway/*.py` (production files directly
-  under `gateway/`, NOT `gateway/tests/`) widens to the full suite
-  with the explicit trigger string `gateway source change (importlib
-  test-loader)`. The bare-name AST resolver (step 5) intentionally
-  excludes `gateway.*` — the importlib loader pattern makes AST
-  edges unreliable there, so the widening trigger remains the stable
-  workaround.
+  shared.egg_logging.signatures import …`, or `from policy import …`
+  rather than `from gateway.policy import …`). Grimp registers
+  modules under fully-qualified names, so a plain grimp traversal
+  misses those edges. **Mitigation:** the bare-name AST resolver
+  (step 5) AST-scans every `.py` and maps bare-name targets back to
+  fully-qualified ids, making these edges visible to narrowing. The
+  resolver covers `gateway.*` even though `gateway/` is not on
+  sys.path during graph build — `gateway/tests/conftest.py`'s
+  `importlib.spec_from_file_location` loader makes every gateway
+  production module bare-name-importable at test time, and the AST
+  resolver only inspects source so the runtime importlib pattern
+  doesn't affect its view.
+
+  Note: the AST resolver only delivers narrowing if the
+  dynamic-import fallback (R6, "dynamic-import reachability") does
+  not fire in its place. Because `_scan_dynamic_imports` regex-scans
+  every module's source for `__import__`, `importlib.util.*`,
+  `SourceFileLoader`, etc., any gateway module that legitimately
+  uses those primitives becomes a seed — and R6 widens for every
+  module reachable through that seed's `find_upstream_modules`
+  closure. To keep `gateway/*.py` edits narrowable, the importlib
+  bootstrap lives in `gateway/_module_loader.py`, a leaf module that
+  imports only stdlib; its upstream closure is empty, so R6 only
+  fires when the bootstrap itself is edited (which is the right
+  call). Keep this invariant in mind when adding any further
+  dynamic-import primitives anywhere in `gateway/` — adding them to
+  a module that is upstream of much of the gateway package would
+  silently re-disable narrowing for everything that flows through
+  it.
 
 These limits are the reason the fallback-trigger list in §4 is as
 broad as it is — narrowing trades coverage for speed, and any
@@ -463,8 +474,6 @@ The stderr trigger string is the diagnostic. Common cases:
   sidecar.
 - `unresolvable baseline` — `origin/main` is missing or
   inaccessible. Check `git remote -v` and `git fetch origin`.
-- `gateway source change (importlib test-loader)` — known blind
-  spot, see §7. The full suite is the right answer here.
 
 **"I want to force the full suite for a single run."**
 

gateway/_module_loader.py

@@ -0,0 +1,61 @@
+"""Sibling-module loader for gateway.gateway.
+
+Isolated into its own file so the ``__import__`` / ``importlib.util``
+primitives below do NOT mark the surrounding ``gateway/gateway.py``
+module as a dynamic-import seed for ``scripts/select_tests.py``.
+
+When ``gateway/gateway.py`` itself was a seed, every ``gateway/<file>.py``
+edit reached the seed transitively through ``find_upstream_modules`` and
+short-circuited ``make test`` to the full suite via the
+``dynamic-import reachability`` trigger.  Moving the importlib helpers
+into this leaf module — which imports only stdlib — keeps the seed
+set small enough that the bare-name AST resolver can actually narrow
+gateway production edits.
+
+Do NOT add gateway-package imports here.  The whole point of this
+file is to keep the seed's ``find_upstream_modules`` closure free of
+``gateway.*`` modules.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import sys
+from pathlib import Path
+from typing import Any
+
+
+def load_sibling_gateway_module(module_name: str) -> Any:
+    """Import a sibling gateway module regardless of test vs prod shape.
+
+    Gateway modules are loaded two ways in this codebase: as a package
+    (``gateway.x``) in production and as flat top-level modules by the
+    test conftest (``__package__ == ""``).  Plain ``import X`` works in
+    production when ``gateway/`` is on ``sys.path``, and in tests when
+    the conftest preloaded ``X`` into ``sys.modules``.  For modules the
+    conftest does *not* preload — like the ones added in #1882 — we
+    fall back to loading the file by explicit path so the features are
+    still exercisable in tests without forcing a conftest edit by the
+    tester role.
+    """
+    mod = sys.modules.get(module_name) or sys.modules.get(f"gateway.{module_name}")
+    if mod is not None:
+        return mod
+    try:
+        mod = __import__(module_name)
+        return mod
+    except ImportError:
+        pass
+    try:
+        mod_path = Path(__file__).parent / f"{module_name}.py"
+        if not mod_path.exists():
+            return None
+        spec = importlib.util.spec_from_file_location(module_name, str(mod_path))
+        if spec is None or spec.loader is None:
+            return None
+        mod = importlib.util.module_from_spec(spec)
+        sys.modules[module_name] = mod
+        spec.loader.exec_module(mod)
+        return mod
+    except Exception:  # pragma: no cover - defensive
+        return None

gateway/gateway.py

@@ -366,42 +366,16 @@
 logger = get_logger("gateway")
 
 
-def _load_sibling_gateway_module(module_name: str) -> Any:
-    """Import a sibling gateway module regardless of test vs prod shape.
-
-    Gateway modules are loaded two ways in this codebase: as a package
-    (``gateway.x``) in production and as flat top-level modules by the
-    test conftest (``__package__ == ""``).  Plain ``import X`` works in
-    production when ``gateway/`` is on ``sys.path``, and in tests when
-    the conftest preloaded ``X`` into ``sys.modules``.  For modules the
-    conftest does *not* preload — like the ones added in #1882 — we
-    fall back to loading the file by explicit path so the features are
-    still exercisable in tests without forcing a conftest edit by the
-    tester role.
-    """
-    mod = sys.modules.get(module_name) or sys.modules.get(f"gateway.{module_name}")
-    if mod is not None:
-        return mod
-    try:
-        mod = __import__(module_name)
-        return mod
-    except ImportError:
-        pass
-    try:
-        import importlib.util
-
-        mod_path = Path(__file__).parent / f"{module_name}.py"
-        if not mod_path.exists():
-            return None
-        spec = importlib.util.spec_from_file_location(module_name, str(mod_path))
-        if spec is None or spec.loader is None:
-            return None
-        mod = importlib.util.module_from_spec(spec)
-        sys.modules[module_name] = mod
-        spec.loader.exec_module(mod)
-        return mod
-    except Exception:  # pragma: no cover - defensive
-        return None
+try:
+    # Production / package mode.
+    from ._module_loader import load_sibling_gateway_module as _load_sibling_gateway_module
+except ImportError:
+    # Standalone-script mode (the test conftest loads gateway.py as
+    # a flat top-level module, in which case the relative import
+    # above raises ImportError before sys.modules has been seeded).
+    from _module_loader import (  # type: ignore[no-redef, import-untyped]
+        load_sibling_gateway_module as _load_sibling_gateway_module,
+    )
 
 
 def _lookup_commit_observer_fn(name: str) -> Any:
@@ -3377,7 +3351,7 @@ def _resolve_checkpoint_token(repo_path: str) -> str | None:
 
 _CHECKPOINT_SCRATCH_DIR = "/home/egg/.egg-worktrees/.checkpoint-scratch"
 
-_checkpoint_scratch_lock = __import__("threading").Lock()
+_checkpoint_scratch_lock = threading.Lock()
 
 
 def _ensure_checkpoint_scratch_repo() -> str | None:

gateway/tests/conftest.py

@@ -88,6 +88,15 @@ def _load_module_with_replaced_imports(
 
 
 # Load modules in dependency order
+# _module_loader holds the importlib bootstrap for gateway.py's
+# sibling-loader.  It has no relative imports.  Loading it here makes
+# `from _module_loader import ...` resolvable in flat-module test
+# mode, mirroring how other gateway dependencies are pre-seeded.
+module_loader = _load_module_with_replaced_imports(
+    "_module_loader",
+    GATEWAY_DIR / "_module_loader.py",
+)
+
 # github_client has no relative imports to other gateway modules
 github_client = _load_module_with_replaced_imports(
     "github_client",
@@ -345,6 +354,7 @@ def _load_module_with_replaced_imports(
     "gateway",
     GATEWAY_DIR / "gateway.py",
     import_replacements={
+        "from ._module_loader import": "from _module_loader import",
         "from .anthropic_credentials import": "from anthropic_credentials import",
         "from .auth import": "from auth import",
         "from .checkpoint_handler import": "from checkpoint_handler import",

scripts/select_tests.py

@@ -146,16 +146,45 @@
 # follow test→production edges in a codebase that grimp alone cannot
 # trace.
 #
-# `gateway.` is intentionally absent: `gateway/` is NOT on sys.path
-# during `build_graph` (the importlib test-loader pattern in
-# `gateway/tests/conftest.py` would shadow grimp's view), and
-# `gateway/*.py` changes are handled by their own dedicated widening
-# trigger.
+# `gateway.` is included even though `gateway/` is NOT on sys.path
+# during `build_graph`: gateway tests reach production through
+# `gateway/tests/conftest.py`'s `_load_module_with_replaced_imports`
+# (importlib `spec_from_file_location`), which makes every gateway
+# production module importable by its bare name (`from policy import
+# X`, `import auth`).  The AST resolver only inspects source — it
+# does not import — so the runtime importlib pattern does not affect
+# its view.  Adding `gateway.` here lets the resolver record the
+# test→production edges that grimp cannot see, replacing the
+# previous blanket "any `gateway/*.py` edit widens to full suite"
+# fallback.
+#
+# Important: parity with `shared.`/`orchestrator.`/`sandbox.` requires
+# that the dynamic-import seed set (R6, `dynamic-import reachability`)
+# does not also pull every gateway production module back into a
+# full-suite fallback.  Because `_scan_dynamic_imports` source-greps
+# for `__import__(`, `importlib.util.spec_from_file_location`, etc.,
+# any gateway module that contains those primitives becomes a seed —
+# and `is_dynamic_import_touched` then widens for every module in
+# that seed's `find_upstream_modules` closure.  When `gateway.gateway`
+# itself was a seed (its source contained `__import__(module_name)`
+# and `__import__("threading")`), the closure spanned ~32 of the 41
+# gateway production modules, so R6 fired in place of the deleted
+# R1 — narrowing in name only.  The fix is to keep the dynamic-import
+# primitives in `gateway/_module_loader.py` (a leaf bootstrap that
+# imports only stdlib), so its upstream closure is empty and R6 only
+# fires when the loader itself is edited.  See
+# `tests/tools/test_select_tests_fallbacks.py::
+# test_gateway_source_change_does_not_widen_with_module_loader_seed`
+# for the regression pin and
+# `tests/tools/test_select_tests_fallbacks.py::
+# test_gateway_source_change_widens_if_gateway_gateway_becomes_seed`
+# for the failure mode being guarded against.
 BARE_NAME_STRIP_PREFIXES: tuple[str, ...] = (
     "shared.",
     "orchestrator.",
     "sandbox.tools.",  # checked before "sandbox." so the longer prefix wins
     "sandbox.",
+    "gateway.",
 )
 
 # Test-root directories (relative paths) the selector emits when
@@ -1286,32 +1315,7 @@ def evaluate_fallback_triggers(
             if _fnmatch(raw_path, pattern):
                 return trigger_string
 
-    # 3d. Gateway importlib-test-loader mapping (R1).  Hits any
-    # `gateway/<file>.py` that is NOT under `gateway/tests/`.  Checked
-    # BEFORE the generic non-.py rule so a mixed diff names the
-    # specific blind spot.
-    #
-    # Layout assumption (locked by current repo as of this PR): gateway/
-    # production source is FLAT — every .py production file is directly
-    # under `gateway/<file>.py`, no subdirectories (verified with
-    # `ls gateway/*.py`).  The TASK-2-3 spec phrases the rule as "any
-    # changed path matching `gateway/*.py`", which is what the
-    # `"/" not in raw_path[len("gateway/") :]` guard implements.  If
-    # gateway production code is ever reorganised into subdirectories
-    # (e.g., `gateway/api/foo.py`), this check would NOT widen on those
-    # subdirectory edits — extend the guard to drop the `"/" not in`
-    # clause at that point.  TASK-5-2's parametrized cases cover the
-    # current flat layout and would catch a change in semantics.
-    for raw_path in paths:
-        if (
-            raw_path.startswith("gateway/")
-            and not raw_path.startswith("gateway/tests/")
-            and "/" not in raw_path[len("gateway/") :]
-            and raw_path.endswith(".py")
-        ):
-            return "gateway source change (importlib test-loader)"
-
-    # 3e. Non-.py changes (decision-5) — the catch-all when none of
+    # 3d. Non-.py changes (decision-5) — the catch-all when none of
     # the more-specific path triggers fired.
     for raw_path in paths:
         if not raw_path.endswith(".py"):