test: restore four silently shadowed definitions and guard against more

Python keeps only the last definition of a name in a scope, so a duplicate
silently deletes the first. Four had accumulated, and two cost real coverage.

tests/agent/test_auxiliary_client.py grew a second _clean_env autouse fixture
alongside an NVIDIA feature. Being module-level with the same name, it
replaced the original for all 158 tests in the file: the ANTHROPIC_API_KEY,
ANTHROPIC_TOKEN, CLAUDE_CODE_OAUTH_TOKEN, OPENAI_MODEL, LLM_MODEL and
NOUS_INFERENCE_BASE_URL stripping went away, and so did the _aux_unhealthy_*
cache reset between tests. Two tests had already started clearing that cache
by hand to work around the leak, one of them with a comment describing the
pollution. The NVIDIA keys are folded into the original fixture and the
duplicate removed.

tests/gateway/test_mattermost.py had two copies of
test_progress_send_with_invalid_thread_root_never_falls_back_flat. The
surviving copy omitted the recorded 400 "invalid root_id" state, so the case
the name describes never ran. Both now run under distinct names.

The other two were harmless but hid the pattern: an exact duplicate in
test_tts_media_routing.py, and a dead _codex_auth_store in
test_credential_pool.py that nothing calls.

tests/test_no_shadowed_test_definitions.py walks every test module and fails
on any repeated definition in one scope, exempting the property/setter/
register family and throwaway _ callbacks. It fails on main listing exactly
these four.

tests/agent/test_auxiliary_client.py

@@ -67,6 +67,7 @@ def _clean_env(monkeypatch):
         "OPENROUTER_API_KEY", "OPENAI_BASE_URL", "OPENAI_API_KEY",
         "OPENAI_MODEL", "LLM_MODEL", "NOUS_INFERENCE_BASE_URL",
         "ANTHROPIC_API_KEY", "ANTHROPIC_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN",
+        "NVIDIA_API_KEY", "NVIDIA_BASE_URL",
     ):
         monkeypatch.delenv(key, raising=False)
     # Module-level unhealthy cache (10-min TTL) leaks between tests;
@@ -3493,16 +3494,6 @@ class TestBuildCallKwargsToolDedup:
 
 
 
-@pytest.fixture(autouse=True)
-def _clean_env(monkeypatch):
-    """Strip provider env vars so each test starts clean."""
-    for key in (
-        "OPENROUTER_API_KEY", "OPENAI_BASE_URL", "OPENAI_API_KEY",
-        "NVIDIA_API_KEY", "NVIDIA_BASE_URL",
-    ):
-        monkeypatch.delenv(key, raising=False)
-
-
 class TestNvidiaBillingHeaders:
     """NVIDIA NIM billing-origin headers are scoped to NVIDIA cloud."""
 

tests/agent/test_credential_pool.py

@@ -1509,23 +1509,6 @@ class TestLeastUsedStrategy:
 
 # ── OpenAI Codex OAuth cross-process sync tests ────────────────────────────
 
-def _codex_auth_store(access: str, refresh: str) -> dict:
-    return {
-        "version": 1,
-        "active_provider": "openai-codex",
-        "providers": {
-            "openai-codex": {
-                "auth_mode": "chatgpt",
-                "tokens": {
-                    "access_token": access,
-                    "refresh_token": refresh,
-                    "id_token": "id-" + access,
-                },
-                "last_refresh": "2026-04-28T00:00:00Z",
-            }
-        },
-    }
-
 
 
 

tests/gateway/test_mattermost.py

@@ -244,8 +244,8 @@ class TestMattermostSend:
 
 
     @pytest.mark.asyncio
-    async def test_progress_send_with_invalid_thread_root_never_falls_back_flat(self):
-        """Tool/status/progress bubbles must stay quiet when the thread is broken."""
+    async def test_progress_send_with_broken_thread_and_no_recorded_error_stays_quiet(self):
+        """Same rule when no post error was recorded: still no flat fallback."""
         self.adapter._reply_mode = "thread"
         self.adapter._api_get = AsyncMock(return_value={"id": "bad_root", "root_id": ""})
         self.adapter._api_post = AsyncMock(return_value={})

tests/gateway/test_tts_media_routing.py

@@ -196,26 +196,3 @@ class _DiscordMediaFailureAdapter(BasePlatformAdapter):
 
     async def get_chat_info(self, chat_id):
         return {"id": chat_id, "type": "dm"}
-
-
-@pytest.mark.asyncio
-async def test_non_streaming_media_failure_notifies_user(tmp_path, monkeypatch):
-    """Attachmentless send_video results must surface a user-visible notice (#66797)."""
-    adapter = _DiscordMediaFailureAdapter()
-    event = _event()
-    media_file = _allowed_media_path(tmp_path, monkeypatch, "clip.mp4")
-    adapter._message_handler = AsyncMock(return_value=f"MEDIA:{media_file}")
-    adapter.send_video = AsyncMock(
-        return_value=SendResult(
-            success=False,
-            error="Discord accepted the message but attached no files (clip.mp4)",
-        )
-    )
-    adapter.send_document = AsyncMock(return_value=SendResult(success=True, message_id="doc"))
-    adapter.send_voice = AsyncMock(return_value=SendResult(success=True, message_id="voice"))
-    adapter.send_multiple_images = AsyncMock()
-
-    await adapter._process_message_background(event, build_session_key(event.source))
-
-    adapter.send_video.assert_awaited_once()
-    assert adapter.notices == ["⚠️ Couldn't deliver the video attachment."]

tests/test_no_shadowed_test_definitions.py

@@ -0,0 +1,124 @@
+"""No test module may define the same name twice in one scope.
+
+Python keeps only the last definition, so a duplicate silently deletes the
+first one. When the shadowed name is a test, its coverage disappears with no
+error and no skip; when it is an ``autouse`` fixture, the whole module quietly
+switches to the newer one's isolation rules.
+
+Both had already happened here:
+
+* ``tests/agent/test_auxiliary_client.py`` grew a second ``_clean_env``
+  ``autouse`` fixture alongside an NVIDIA feature. It replaced the original for
+  all 158 tests in the file, dropping the ``ANTHROPIC_API_KEY`` /
+  ``ANTHROPIC_TOKEN`` / ``CLAUDE_CODE_OAUTH_TOKEN`` env stripping and the
+  ``_aux_unhealthy_*`` cache reset between tests. Individual tests had started
+  clearing that cache by hand to work around it.
+* ``tests/gateway/test_mattermost.py`` had two copies of
+  ``test_progress_send_with_invalid_thread_root_never_falls_back_flat``; the
+  surviving one omitted the recorded 400 ``invalid root_id`` state, so the
+  case the name describes was never exercised.
+
+This guard is cheap and catches the whole class at collection time.
+"""
+
+from __future__ import annotations
+
+import ast
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+TESTS_ROOT = REPO_ROOT / "tests"
+
+# Decorators that legitimately repeat a name in one scope.
+_REDEFINING_DECORATORS = ("overload", "setter", "getter", "deleter", "register")
+
+# Throwaway callbacks conventionally named `_` are not shadowing bugs.
+_ALLOWED_REPEATS = {"_"}
+
+
+def _decorator_names(node: ast.AST) -> list[str]:
+    out = []
+    for dec in getattr(node, "decorator_list", []):
+        try:
+            out.append(ast.unparse(dec))
+        except Exception:  # pragma: no cover - defensive
+            pass
+    return out
+
+
+def _duplicates_in(body, scope: str, rel: str) -> list[str]:
+    seen: dict[str, int] = {}
+    problems: list[str] = []
+    for node in body:
+        if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
+            continue
+        if node.name in _ALLOWED_REPEATS:
+            continue
+        if any(
+            marker in dec
+            for dec in _decorator_names(node)
+            for marker in _REDEFINING_DECORATORS
+        ):
+            seen[node.name] = node.lineno
+            continue
+        if node.name in seen:
+            problems.append(
+                f"{rel}:{node.lineno} {scope}.{node.name}() shadows the "
+                f"definition at line {seen[node.name]}"
+            )
+        seen[node.name] = node.lineno
+    return problems
+
+
+def _test_modules() -> list[Path]:
+    return sorted(TESTS_ROOT.rglob("test_*.py"))
+
+
+def test_no_shadowed_definitions_in_test_modules():
+    problems: list[str] = []
+    for path in _test_modules():
+        try:
+            tree = ast.parse(path.read_text(encoding="utf-8"))
+        except (SyntaxError, UnicodeDecodeError):  # pragma: no cover
+            continue
+        rel = path.relative_to(REPO_ROOT).as_posix()
+        problems += _duplicates_in(tree.body, "<module>", rel)
+        for node in ast.walk(tree):
+            if isinstance(node, ast.ClassDef):
+                problems += _duplicates_in(node.body, node.name, rel)
+
+    assert not problems, (
+        "A duplicate definition silently deletes the earlier one. Rename or "
+        "remove:\n  " + "\n  ".join(problems)
+    )
+
+
+def test_guard_detects_a_known_duplicate_shape():
+    """The guard must actually fire, not vacuously pass."""
+    src = (
+        "def test_a():\n    pass\n\n"
+        "def test_a():\n    pass\n"
+    )
+    tree = ast.parse(src)
+    assert _duplicates_in(tree.body, "<module>", "fake.py")
+
+
+@pytest.mark.parametrize("decorator", ["@property", "@x.setter", "@functools.singledispatch"])
+def test_guard_allows_legitimate_redefinition(decorator):
+    src = (
+        "class C:\n"
+        "    def f(self):\n        pass\n"
+        f"    {decorator}\n"
+        "    def f(self):\n        pass\n"
+    )
+    tree = ast.parse(src)
+    cls = tree.body[0]
+    problems = _duplicates_in(cls.body, "C", "fake.py")
+    # Only the setter/getter/register family is exempt; a bare @property
+    # repeat is still a real shadow.
+    if any(m in decorator for m in _REDEFINING_DECORATORS):
+        assert not problems
+    else:
+        assert problems