From 3dd8059a05dc4ce0616aaeb5fb5c63cd7f6b2178 Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Wed, 29 Jul 2026 12:18:06 -0700
Subject: [PATCH] =?UTF-8?q?fix(tests):=20eliminate=20flaky/broken=20tests?=
 =?UTF-8?q?=20=E2=80=94=20shadow=20sys.path=20inserts,=20unmocked=20networ?=
 =?UTF-8?q?k=20in=20compressor=20tests,=20stale-SDK=20feishu=20pin=20guard?=
 =?UTF-8?q?,=20quadratic=20redact=20regexes?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Remove tests/-shadowing sys.path.insert(dirname/'..') from 11 test files:
  it prepended the tests/ dir itself to sys.path, so 'import agent' /
  'import hermes_cli' resolved to the test packages and collection died
  with ModuleNotFoundError depending on import order (2 files failed in
  every full-suite run; 9 more were latent).
- Patch call_llm in 5 context-compressor tests that called compress()
  unmocked: each burned ~50s attempting live LLM traffic through the
  relay before falling back (572s file — the slowest in the suite, and
  flaky under the 300s per-file timeout). File now runs in ~5s.
- agent/redact.py: fix two catastrophically-backtracking regexes hit by
  the compressor's redaction pass on large payloads —
  _STRICT_URL_USERINFO_RE anchors on the mandatory '//' (optional-scheme
  prefix backtracked O(n^2): ~55s on a 320KB payload, now sub-ms;
  output-equivalence fuzz-verified on 20k random strings), and the
  _CFG_DOTTED_RE/_CFG_ANCHORED_RE subs gain an exact linear keyword
  pre-gate so secret-free text skips the quadratic pattern entirely.
- tests/gateway/test_feishu.py: version-guard the extra_ua_tags SDK
  signature check; the repo pins lark-oapi==1.6.8 but stale local
  installs (1.5.3) fail the assertion — skip below the pin.
- tests/tools/test_managed_browserbase_and_modal.py: stub
  agent.redact + agent.credential_persistence in the fake agent package
  (empty __path__ blocks all real agent.* imports added since the fake
  was written).
- tests/gateway/test_startup_restart_race.py: raise wait_for timeouts
  2s -> 30s; 2s wall-clock on a loaded 40-worker box flaked in the
  baseline run (passes instantly when the box is quiet).
---
 agent/redact.py                               | 26 +++++++++++++++++--
 tests/agent/test_context_compressor.py        | 25 +++++++++++-------
 tests/agent/test_model_metadata_local_ctx.py  |  3 ---
 tests/agent/test_model_metadata_ssl.py        |  3 ---
 tests/agent/test_probe_cache_followups.py     |  3 ---
 tests/cli/test_cli_init.py                    |  1 -
 tests/cli/test_cli_user_message_preview.py    |  2 --
 tests/cli/test_resume_display.py              |  3 ---
 tests/cli/test_tool_progress_scrollback.py    |  2 --
 tests/gateway/test_feishu.py                  | 19 +++++++++++++-
 tests/gateway/test_startup_restart_race.py    |  8 +++---
 tests/hermes_cli/test_auth_ssl_macos.py       |  2 --
 tests/hermes_cli/test_voice_wrapper.py        |  3 ---
 tests/test_ctx_halving_fix.py                 |  3 ---
 tests/test_model_picker_scroll.py             |  3 ---
 .../test_managed_browserbase_and_modal.py     | 11 ++++++++
 16 files changed, 73 insertions(+), 44 deletions(-)

diff --git a/agent/redact.py b/agent/redact.py
index bff23934da6..b104ad4c22f 100644
--- a/agent/redact.py
+++ b/agent/redact.py
@@ -140,6 +140,11 @@ _ENV_ASSIGN_RE = re.compile(
 # The colon-form URL guard (skip when ``://`` present) lives at the call site.
 _SECRET_CFG_NAMES = r"(?:api[ _.\-]?key|token|secret|passwd|password|credential|auth)"
 _CFG_VALUE = r"(['\"]?)([^\s&]+?)\2(?=[\s&]|$)"
+# Linear pre-gate for the _CFG_*_RE subs below: a text with no secret keyword
+# can never match either pattern, so the (potentially backtrack-heavy) subs
+# are skipped entirely for such text. See the call site in
+# redact_sensitive_text().
+_CFG_SECRET_WORD_RE = re.compile(_SECRET_CFG_NAMES, re.IGNORECASE)
 
 # Programmatic env lookups (``os.getenv(...)``, ``os.environ[...]``,
 # ``os.environ.get(...)``, ``process.env.X``, ``$ENV{X}``) reference variable
@@ -377,8 +382,17 @@ _STRICT_URL_PARAM_RE = re.compile(
 # Match userinfo in both absolute (``scheme://user:pass@host``) and
 # network-path (``//user:pass@host``) references. The authority boundary stops
 # at path/query/fragment delimiters so an ``@`` elsewhere in a URL is ignored.
+#
+# Anchored on the mandatory ``//`` rather than an optional scheme prefix: the
+# scheme sits outside the match either way (replacement callbacks re-emit
+# group(1), so ``https:`` stays untouched in the surrounding text), and the
+# old optional-scheme prefix ``(?:[A-Za-z][A-Za-z0-9+.-]*:)?`` backtracked
+# catastrophically (O(n²)) on long unbroken alphanumeric runs — a 320KB
+# synthetic compaction payload spent ~55s inside this pattern per sub() call.
+# Output-equivalence to the old pattern was fuzz-verified (20k random strings
+# plus targeted URL forms).
 _STRICT_URL_USERINFO_RE = re.compile(
-    r"((?:[A-Za-z][A-Za-z0-9+.-]*:)?//)([^/\s?#@]+)@"
+    r"(//)([^/\s?#@]+)@"
 )
 
 # HTTP access logs often use a relative request target rather than a full URL:
@@ -706,7 +720,15 @@ def redact_sensitive_text(
             # web-URL query params are intentionally passed through (see note
             # near the bottom of this function); _DB_CONNSTR_RE still guards
             # connection-string passwords.
-            if "://" not in text:
+            #
+            # Extra gate: every _CFG_*_RE match requires a secret keyword in
+            # the key, so a text without any secret keyword cannot match —
+            # skipping is exact. This matters because _CFG_DOTTED_RE
+            # backtracks quadratically on long unbroken [A-Za-z0-9_.\-] runs
+            # (e.g. base64/hex blobs in compaction payloads); the linear
+            # keyword scan prevents that pathological path on secret-free
+            # text.
+            if "://" not in text and _CFG_SECRET_WORD_RE.search(text):
                 text = _CFG_DOTTED_RE.sub(_redact_env, text)
                 text = _CFG_ANCHORED_RE.sub(_redact_env, text)
 
diff --git a/tests/agent/test_context_compressor.py b/tests/agent/test_context_compressor.py
index 88c694515d1..3d08ad46932 100644
--- a/tests/agent/test_context_compressor.py
+++ b/tests/agent/test_context_compressor.py
@@ -174,7 +174,8 @@ class TestCompress:
 
     def test_too_few_messages_returns_unchanged(self, compressor):
         msgs = self._make_messages(4)  # protect_first=2 + protect_last=2 + 1 = 5 needed
-        result = compressor.compress(msgs)
+        with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
+            result = compressor.compress(msgs)
         assert result == msgs
 
     def test_truncation_fallback_no_client(self, compressor):
@@ -380,15 +381,19 @@ class TestCompress:
     def test_compression_increments_count(self, compressor):
         msgs = self._make_messages(10)
         # Default config (abort_on_summary_failure=False) — fallback path
-        # increments the count even on summary failure.
-        compressor.compress(msgs)
-        assert compressor.compression_count == 1
-        compressor.compress(msgs)
-        assert compressor.compression_count == 2
+        # increments the count even on summary failure. Patch call_llm so the
+        # summary attempt fails fast instead of attempting live network I/O
+        # (unpatched, each compress() call burned ~50s in connect/retry).
+        with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
+            compressor.compress(msgs)
+            assert compressor.compression_count == 1
+            compressor.compress(msgs)
+            assert compressor.compression_count == 2
 
     def test_protects_first_and_last(self, compressor):
         msgs = self._make_messages(10)
-        result = compressor.compress(msgs)
+        with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
+            result = compressor.compress(msgs)
         # First 2 messages should be preserved (protect_first_n=2)
         # Last 2 messages should be preserved (protect_last_n=2)
         assert result[-1]["content"] == msgs[-1]["content"]
@@ -607,7 +612,8 @@ class TestGenerateSummaryNoneContent:
             {"role": "user" if i % 2 == 0 else "assistant", "content": f"msg {i}"}
             for i in range(10)
         ]
-        result = c.compress(msgs)
+        with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
+            result = c.compress(msgs)
         assert len(result) < len(msgs)
 
 
@@ -2682,7 +2688,8 @@ class TestSummaryTargetRatio:
             + [{"role": "user" if i % 2 == 0 else "assistant", "content": f"msg {i}"}
                for i in range(8)]
         )
-        result = c.compress(msgs)
+        with patch("agent.context_compressor.call_llm", side_effect=RuntimeError("no provider")):
+            result = c.compress(msgs)
         # System prompt (msg[0]) survives as head
         assert result[0]["role"] == "system"
         assert result[0]["content"].startswith("System prompt")
diff --git a/tests/agent/test_model_metadata_local_ctx.py b/tests/agent/test_model_metadata_local_ctx.py
index 1fcc24ecf82..a1fa3c40dc1 100644
--- a/tests/agent/test_model_metadata_local_ctx.py
+++ b/tests/agent/test_model_metadata_local_ctx.py
@@ -4,13 +4,10 @@ get_model_context_length.
 All tests use synthetic inputs — no filesystem or live server required.
 """
 
-import sys
-import os
 from unittest.mock import MagicMock, patch
 
 import pytest
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 @pytest.fixture(autouse=True)
diff --git a/tests/agent/test_model_metadata_ssl.py b/tests/agent/test_model_metadata_ssl.py
index 6859fd309cd..f54bd9a7777 100644
--- a/tests/agent/test_model_metadata_ssl.py
+++ b/tests/agent/test_model_metadata_ssl.py
@@ -9,11 +9,8 @@ No filesystem or network I/O required — we use tmp_path to create real
 CA bundle stand-in files and monkeypatch env vars.
 """
 
-import os
-import sys
 from pathlib import Path
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 import pytest
 
diff --git a/tests/agent/test_probe_cache_followups.py b/tests/agent/test_probe_cache_followups.py
index 7d87516a407..b6659db86a6 100644
--- a/tests/agent/test_probe_cache_followups.py
+++ b/tests/agent/test_probe_cache_followups.py
@@ -7,13 +7,10 @@ Covers:
 
 from __future__ import annotations
 
-import os
-import sys
 from unittest.mock import MagicMock, patch
 
 import pytest
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 @pytest.fixture(autouse=True)
diff --git a/tests/cli/test_cli_init.py b/tests/cli/test_cli_init.py
index c3915777da0..0f50ff6da22 100644
--- a/tests/cli/test_cli_init.py
+++ b/tests/cli/test_cli_init.py
@@ -6,7 +6,6 @@ import sys
 from types import SimpleNamespace
 from unittest.mock import MagicMock, patch
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 def _make_cli(env_overrides=None, config_overrides=None, **kwargs):
diff --git a/tests/cli/test_cli_user_message_preview.py b/tests/cli/test_cli_user_message_preview.py
index f3e84759eed..b3829587d06 100644
--- a/tests/cli/test_cli_user_message_preview.py
+++ b/tests/cli/test_cli_user_message_preview.py
@@ -1,9 +1,7 @@
 import importlib
-import os
 import sys
 from unittest.mock import MagicMock, patch
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 _cli_mod = None
diff --git a/tests/cli/test_resume_display.py b/tests/cli/test_resume_display.py
index 6f36ace71ae..8a314746fda 100644
--- a/tests/cli/test_resume_display.py
+++ b/tests/cli/test_resume_display.py
@@ -5,14 +5,11 @@ Verifies that resuming a session shows a compact recap of the previous
 conversation with correct formatting, truncation, and config behavior.
 """
 
-import os
-import sys
 from io import StringIO
 from unittest.mock import MagicMock, patch
 
 import cli as cli_mod
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 def _make_cli(config_overrides=None, env_overrides=None, **kwargs):
diff --git a/tests/cli/test_tool_progress_scrollback.py b/tests/cli/test_tool_progress_scrollback.py
index 2f5f6a8952d..00033bb4b11 100644
--- a/tests/cli/test_tool_progress_scrollback.py
+++ b/tests/cli/test_tool_progress_scrollback.py
@@ -5,12 +5,10 @@ persistent lines to scrollback on tool.completed, restoring the stacked
 tool history that was lost when the TUI switched to a single-line spinner.
 """
 
-import os
 import sys
 import importlib
 from unittest.mock import MagicMock, patch
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 # Module-level reference to the cli module (set by _make_cli on first call)
 _cli_mod = None
diff --git a/tests/gateway/test_feishu.py b/tests/gateway/test_feishu.py
index d887bc94f38..480b6a65dce 100644
--- a/tests/gateway/test_feishu.py
+++ b/tests/gateway/test_feishu.py
@@ -176,9 +176,26 @@ class TestFeishuMessageNormalization(unittest.TestCase):
 class TestFeishuAdapterMessaging(unittest.TestCase):
     @unittest.skipUnless(_HAS_LARK_OAPI, "lark-oapi not installed")
     def test_websocket_sdk_accepts_channel_ua_tag(self):
-        """The shipped SDK must support the Channel signaling argument."""
+        """The shipped SDK must support the Channel signaling argument.
+
+        Guarded on the pinned version: the repo pins lark-oapi==1.6.8 (the
+        first release with ``extra_ua_tags``). Dev machines can carry an
+        older lazy-installed lark-oapi that predates the argument — that is
+        an environment artifact, not a product regression, so skip rather
+        than fail there. Environments installing the pin (the feishu extra)
+        still exercise the real assertion.
+        """
         import inspect
 
+        from importlib.metadata import version as _pkg_version
+
+        installed = tuple(int(p) for p in _pkg_version("lark-oapi").split(".")[:3] if p.isdigit())
+        if installed < (1, 6, 8):
+            self.skipTest(
+                f"lark-oapi {_pkg_version('lark-oapi')} predates extra_ua_tags; "
+                "repo pin is 1.6.8 — stale local install"
+            )
+
         from lark_oapi.ws import Client as FeishuWSClient
 
         signature = inspect.signature(FeishuWSClient)
diff --git a/tests/gateway/test_startup_restart_race.py b/tests/gateway/test_startup_restart_race.py
index bdf5bbc42a4..3c1666ac830 100644
--- a/tests/gateway/test_startup_restart_race.py
+++ b/tests/gateway/test_startup_restart_race.py
@@ -136,7 +136,7 @@ async def test_startup_aborts_when_restart_requested_before_start(tmp_path, monk
     runner.request_restart(detached=False, via_service=True)
     runner._create_adapter = MagicMock()
 
-    result = await asyncio.wait_for(runner.start(), timeout=2)
+    result = await asyncio.wait_for(runner.start(), timeout=30)
 
     assert result is True
     runner._create_adapter.assert_not_called()
@@ -167,7 +167,7 @@ async def test_startup_aborts_when_restart_begins_during_platform_connect(tmp_pa
     telegram.disconnect = disconnect_and_release
     runner._create_adapter = MagicMock(side_effect=[telegram, slack])
 
-    result = await asyncio.wait_for(runner.start(), timeout=2)
+    result = await asyncio.wait_for(runner.start(), timeout=30)
 
     assert result is True
     assert telegram.disconnected is True
@@ -202,7 +202,7 @@ async def test_startup_abort_waits_for_existing_stop_task(tmp_path):
 
     result = await asyncio.wait_for(
         runner._abort_startup_if_shutdown_requested(adapter, Platform.TELEGRAM),
-        timeout=2,
+        timeout=30,
     )
 
     assert result is True
@@ -227,7 +227,7 @@ async def test_startup_aborts_after_registered_adapter_restart(tmp_path, monkeyp
 
     runner._update_platform_runtime_status = MagicMock(side_effect=update_platform_runtime_status)
 
-    result = await asyncio.wait_for(runner.start(), timeout=2)
+    result = await asyncio.wait_for(runner.start(), timeout=30)
 
     assert result is True
     assert telegram.connected is True
diff --git a/tests/hermes_cli/test_auth_ssl_macos.py b/tests/hermes_cli/test_auth_ssl_macos.py
index a6ebb316814..8e6c0d062f5 100644
--- a/tests/hermes_cli/test_auth_ssl_macos.py
+++ b/tests/hermes_cli/test_auth_ssl_macos.py
@@ -10,7 +10,6 @@ fixture because `ssl.create_default_context(cafile=...)` parses the
 bundle and refuses stubs.
 """
 
-import os
 import shutil
 import ssl
 import subprocess
@@ -19,7 +18,6 @@ from pathlib import Path
 
 import pytest
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 from hermes_cli.auth import _default_verify, _resolve_verify
 
diff --git a/tests/hermes_cli/test_voice_wrapper.py b/tests/hermes_cli/test_voice_wrapper.py
index a403d3c2892..eb0b90d9bbf 100644
--- a/tests/hermes_cli/test_voice_wrapper.py
+++ b/tests/hermes_cli/test_voice_wrapper.py
@@ -9,12 +9,9 @@ and ``speak_text`` tolerates empty input without touching the provider
 stack.
 """
 
-import os
-import sys
 
 import pytest
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 class TestPublicAPI:
diff --git a/tests/test_ctx_halving_fix.py b/tests/test_ctx_halving_fix.py
index 63c965ac965..6cbe4af8851 100644
--- a/tests/test_ctx_halving_fix.py
+++ b/tests/test_ctx_halving_fix.py
@@ -23,11 +23,8 @@ These are different and the old code conflated them; the fix keeps them
 separate.
 """
 
-import sys
-import os
 from unittest.mock import MagicMock
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 
diff --git a/tests/test_model_picker_scroll.py b/tests/test_model_picker_scroll.py
index f37a82fe611..3a30580299d 100644
--- a/tests/test_model_picker_scroll.py
+++ b/tests/test_model_picker_scroll.py
@@ -12,10 +12,7 @@ is always within the visible window.  These tests exercise that logic in
 isolation without requiring a real TTY.
 """
 
-import sys
-import os
 
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 
 # ---------------------------------------------------------------------------
diff --git a/tests/tools/test_managed_browserbase_and_modal.py b/tests/tools/test_managed_browserbase_and_modal.py
index 96ab53ae090..f86d8d748b6 100644
--- a/tests/tools/test_managed_browserbase_and_modal.py
+++ b/tests/tools/test_managed_browserbase_and_modal.py
@@ -101,6 +101,17 @@ def _install_fake_tools_package():
     sys.modules["agent.auxiliary_client"] = types.SimpleNamespace(
         call_llm=lambda *args, **kwargs: "",
     )
+    # The fake `agent` package has an empty __path__, so every real
+    # agent.* submodule that production code imports needs an explicit
+    # stand-in here. tools.browser_tool imports redact_cdp_url;
+    # hermes_cli.auth (imported transitively by nous_account /
+    # tool_backend_helpers) imports sanitize_borrowed_credential_payload.
+    sys.modules["agent.redact"] = types.SimpleNamespace(
+        redact_cdp_url=lambda value: str(value),
+    )
+    sys.modules["agent.credential_persistence"] = types.SimpleNamespace(
+        sanitize_borrowed_credential_payload=lambda entry, provider_id=None: entry,
+    )
 
     # Stubs for the browser-provider plugin layer introduced in PR #25214.
     # The fake `agent` package has an empty __path__ so real submodules