test(unit): fix cross_account pollution + standardize unit-test setup

docs/development.md

@@ -105,6 +105,17 @@ These three rules prevent the most common behavioral regressions:
 
 `DEPLOY_MODE` in `.env` gates build phases via `DEPLOY_FLAG_*` flags and maps to Terraform `deploy_*` vars. Modes: `full` (default), `agents-only` (skips Next.js/S3/CloudFront), `gateway-only`, `tools-only`. `DEPLOY_TOOLS` filters Lambda tools by `tools.json` keys. `GATEWAY_AUTH` is `iam` (default) or `oauth` (adds Cognito JWT authorizer for external clients).
 
+## Testing Conventions
+
+Unit tests live in `tests/unit/` (run by `make test-unit` alongside `tests/topology/`); moto/DynamoDB fixtures are shared via `tests/conftest.py`; package-wide setup is in `tests/unit/conftest.py`. Follow these so new tests don't drift:
+
+- **Loading a Lambda handler**: use `importlib.util.spec_from_file_location(...)` + `exec_module` under a **namespaced** module name (e.g. `lambda_runtime_handler`, not `handler`) so multiple `handler.py` files don't collide. Reference: `tests/unit/test_tag_governance_tool.py`.
+- **NEVER stub `shared.*` (or any real package) via a module-scope `sys.modules[...] = MagicMock()`.** It never gets removed and pollutes every later test file (this once broke all of `test_cross_account.py`). Handlers import `shared.cross_account` **lazily inside functions**, so `exec_module` does not need it stubbed to import. If a test exercises that path, stub it in a **fixture** with `monkeypatch.setitem(sys.modules, ...)` (auto-restored) — see the `cross_account_stub` fixture in `test_lambda_runtime_tool.py`.
+- **AWS env**: don't set `AWS_REGION`/credentials per file. `tests/unit/conftest.py` sets deterministic region + dummy creds for the whole package at import time. Only set genuinely test-specific env (e.g. a tool's own `*_TABLE_NAME`) in the file, and prefer `monkeypatch.setenv` inside a test over module-scope mutation.
+- **Mocking**: DynamoDB-heavy tests use **moto** (`mock_aws` via the shared fixtures); handler-routing/logic tests use **`MagicMock` + `monkeypatch`** on the handler's own seams (e.g. `monkeypatch.setattr(handler, "_get_lambda_client", ...)`). Don't reach into `sys.modules` to patch — patch the handler reference.
+- **Markers**: `tests/unit/conftest.py` auto-applies the `unit` marker to everything under `tests/unit/`, so `pytest -m unit` selects the whole suite. No per-file `pytestmark` needed.
+- **Prove a regression test fails on the bug first** (stash the fix → test fails → restore → passes) — see the report-mode example in `temp/PR-PLAYBOOK.md`.
+
 ## Observability
 
 Runtime → X-Ray trace delivery and runtime → CWL application-log delivery are wired in `terraform/modules/core/observability/main.tf`. Spans land in `aws/spans`; application logs vend to `/aws/vendedlogs/bedrock-agentcore/runtime/{project}`. Runtime execution roles (both `agent-runtime-base` and `agentcore-runtime`) grant four X-Ray actions — `PutTraceSegments`, `PutTelemetryRecords`, `GetSamplingRules`, `GetSamplingTargets`. The two `Get*` are load-bearing: ADOT's sampler calls them every ~10s; without them spans get silently dropped.

tests/unit/conftest.py

@@ -0,0 +1,47 @@
+"""Shared setup for the unit-test package.
+
+Two things are centralized here so individual test modules don't each
+re-implement them (which is how conventions drift — see the sys.modules
+pollution that once lived in test_lambda_runtime_tool.py):
+
+1. Deterministic AWS env for import time. Several Lambda handlers read
+   AWS_REGION (and clients like MemoryClient default to us-west-2 if it is
+   unset — see CLAUDE.md), and they are imported at test-collection time via
+   importlib. conftest.py is imported BEFORE the test modules in its
+   directory, so setting these at module scope here guarantees they are in
+   place before any handler import. Dummy credentials ensure a stray boto3
+   call can never reach a real account from the unit suite.
+
+2. Automatic `unit` marker. Every test under tests/unit/ is a unit test, so
+   rather than repeat `pytestmark = pytest.mark.unit` in 15 files (only one
+   did), we tag them all here. This makes `pytest -m unit` actually select
+   the whole unit suite.
+"""
+
+import os
+
+import pytest
+
+# --- 1. Deterministic AWS env (module scope → runs at conftest import,
+#        before test modules import their handlers) --------------------------
+os.environ.setdefault("AWS_REGION", "us-east-1")
+os.environ.setdefault("AWS_DEFAULT_REGION", "us-east-1")
+# Dummy creds so no unit test can accidentally hit a real account. setdefault
+# means a developer who has real creds exported still gets them overridden
+# only if unset — but the unit suite never makes live calls, so this is a
+# backstop, not a dependency.
+os.environ.setdefault("AWS_ACCESS_KEY_ID", "testing")
+os.environ.setdefault("AWS_SECRET_ACCESS_KEY", "testing")
+os.environ.setdefault("AWS_SESSION_TOKEN", "testing")
+
+
+# --- 2. Auto-apply the `unit` marker to everything in this package ----------
+def pytest_collection_modifyitems(config, items):
+    """Tag every test collected under tests/unit/ with the `unit` marker,
+    so `pytest -m unit` selects the full suite without each file having to
+    declare `pytestmark = pytest.mark.unit`.
+    """
+    unit_root = os.path.dirname(__file__)
+    for item in items:
+        if str(item.fspath).startswith(unit_root):
+            item.add_marker(pytest.mark.unit)

tests/unit/test_health_events_collector.py

@@ -26,8 +26,9 @@
 
 _REPO_ROOT = Path(__file__).resolve().parents[2]
 
+# AWS_REGION is set for the whole unit package in tests/unit/conftest.py.
+# These two are test-specific config the collector reads at import time.
 os.environ.setdefault("HEALTH_EVENTS_TABLE_NAME", "test-table")
-os.environ.setdefault("AWS_REGION", "us-east-1")
 # Disable LLM by default so tests don't hit Bedrock. Individual tests opt in.
 os.environ["ENRICHMENT_MODEL_ID"] = ""
 

tests/unit/test_lambda_runtime_tool.py

@@ -30,20 +30,25 @@
 
 _REPO_ROOT = Path(__file__).resolve().parents[2]
 
-# Default region before importing the handler.
-os.environ.setdefault("AWS_REGION", "us-east-1")
+# AWS_REGION is set for the whole unit package in tests/unit/conftest.py
+# (imported before this module), so the handler sees it at import time.
 
 # Load the handler under a namespaced module name so it doesn't collide with
-# other `handler.py` modules.
+# other `handler.py` modules (network-resilience, collectors, etc.).
+#
+# NOTE: the handler imports `shared.cross_account` LAZILY (inside functions,
+# not at module scope), so exec_module succeeds without stubbing `shared`.
+# Tests that exercise those code paths request the `cross_account_stub`
+# fixture below — we must NOT install a global sys.modules["shared.cross_account"]
+# here, because a module-scope stub that is never removed pollutes the whole
+# test session and breaks the real tests/unit/test_cross_account.py.
 _HANDLER_PATH = _REPO_ROOT / "src" / "lambda" / "mcp" / "lambda-runtime" / "handler.py"
 _spec = importlib.util.spec_from_file_location("lambda_runtime_handler", _HANDLER_PATH)
 handler = importlib.util.module_from_spec(_spec)
-
-# Mock the shared.cross_account import that happens at module level
-sys.modules["shared"] = MagicMock()
-sys.modules["shared.cross_account"] = MagicMock()
 _spec.loader.exec_module(handler)
-# Register the module so patch() can resolve it
+# Register the module under its own name so patch("lambda_runtime_handler...")
+# targets resolve. This is namespaced to this handler and does not shadow any
+# real package, so it is safe to leave in sys.modules.
 sys.modules["lambda_runtime_handler"] = handler
 
 
@@ -52,6 +57,23 @@
 # ---------------------------------------------------------------------------
 
 
+@pytest.fixture
+def cross_account_stub(monkeypatch):
+    """Provide a scoped stub for `shared.cross_account` for the tests that
+    exercise the handler's lazy `from shared.cross_account import get_aws_client`.
+
+    monkeypatch.setitem restores the previous sys.modules state (including
+    absence) on teardown, so this never leaks into other test files — unlike
+    a bare module-level `sys.modules["shared.cross_account"] = MagicMock()`.
+    Returns the stub module so the test can set `get_aws_client` on it.
+    """
+    shared_pkg = sys.modules.get("shared") or MagicMock()
+    stub = MagicMock()
+    monkeypatch.setitem(sys.modules, "shared", shared_pkg)
+    monkeypatch.setitem(sys.modules, "shared.cross_account", stub)
+    return stub
+
+
 def _make_context(tool_name: str) -> SimpleNamespace:
     """Build the AgentCore Gateway context shape the dispatcher reads."""
     return SimpleNamespace(
@@ -446,7 +468,7 @@ def mock_client_factory(region=None):
 
 
 class TestDiscoverLambdaRegions:
-    def test_success(self, monkeypatch):
+    def test_success(self, monkeypatch, cross_account_stub):
         """Discovers regions with Lambda functions."""
         mock_ec2 = MagicMock()
         mock_ec2.describe_regions.return_value = {
@@ -474,17 +496,15 @@ def mock_get_client(service, region_name=None):
             client.get_paginator.return_value = paginator
             return client
 
-        # Patch the shared.cross_account.get_aws_client used inside
+        # Stub the shared.cross_account.get_aws_client the handler imports lazily.
+        cross_account_stub.get_aws_client = mock_get_client
         with patch("lambda_runtime_handler.boto3"):
-            monkeypatch.setattr(
-                sys.modules["shared.cross_account"], "get_aws_client", mock_get_client
-            )
             result = handler.handle_discover_lambda_regions({})
 
         assert result["total_functions"] == 2
         assert result["total_regions"] == 1  # only us-east-1 has functions
 
-    def test_region_scan_error_handled(self, monkeypatch):
+    def test_region_scan_error_handled(self, monkeypatch, cross_account_stub):
         """A region that throws during scan doesn't crash discovery."""
         mock_ec2 = MagicMock()
         mock_ec2.describe_regions.return_value = {
@@ -496,10 +516,8 @@ def mock_get_client(service, region_name=None):
                 return mock_ec2
             raise Exception("Connection timeout")
 
+        cross_account_stub.get_aws_client = mock_get_client
         with patch("lambda_runtime_handler.boto3"):
-            monkeypatch.setattr(
-                sys.modules["shared.cross_account"], "get_aws_client", mock_get_client
-            )
             result = handler.handle_discover_lambda_regions({})
 
         # Should return empty results, not crash

tests/unit/test_tag_governance_tool.py

@@ -30,8 +30,8 @@
 
 _REPO_ROOT = Path(__file__).resolve().parents[2]
 
-# Default region before importing the handler (it reads AWS_REGION at import).
-os.environ.setdefault("AWS_REGION", "us-east-1")
+# AWS_REGION is set for the whole unit package in tests/unit/conftest.py
+# (imported before this module), so the handler sees it at import time.
 
 # Load the handler under a namespaced module name so it doesn't collide with
 # other `handler.py` modules (network-resilience, collectors, etc.).

tests/unit/test_topology.py

@@ -35,7 +35,8 @@
 
 import pytest
 
-pytestmark = pytest.mark.unit
+# The `unit` marker is applied to every test under tests/unit/ by
+# tests/unit/conftest.py — no per-file pytestmark needed.
 
 _REPO_ROOT = Path(__file__).resolve().parents[2]
 _HIERARCHY_PATH = _REPO_ROOT / "src" / "agents" / "hierarchy.json"