From fd2887e8dcd18a579d72d32e4a414675294b8c1f Mon Sep 17 00:00:00 2001 From: Robert Sigmundsson <230784065+RobertSigmundsson@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:12:18 +0200 Subject: [PATCH] fix(cli): the update check honours SURREAL_MEMORY_NO_UPDATE_CHECK, and the suite sets it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run_update_check_background() had no opt-out. Most CLI invocations started a daemon thread that queried PyPI — the callback skips a short list of commands and --json, but not the several hundred invocations the test suite makes — so a full run on a machine with egress made real outbound calls to pypi.org, and on a machine without one paid the connection timeout instead. It also feeds a flake. The banner itself goes to stderr, deliberately and with a comment saying why, so it does not break a piped `smem recall … | jq`. But click's CliRunner merges stderr into result.output, and the CLI tests parse that as JSON — so the banner lands in a different test's parse each run. The check now short-circuits when SURREAL_MEMORY_NO_UPDATE_CHECK is truthy, and tests/conftest.py sets it alongside the $HOME redirect that #110 and #121 already put there. Same fixture, same reason: a test run should not reach out of the machine it runs on. Truthiness deliberately follows unified_config._env_truthy, the convention this repo already uses for SURREAL_MEMORY_EMBEDDING_ENABLED, SURREAL_MEMORY_SYNC_ENABLED and the SURREAL_MEMORY_REASONING_* switches: 1/true/yes/on, case-insensitive. Everything else — including off, no, 0 and false — leaves the check running. A fourth, inverted convention where any non-empty value disabled it would mean SURREAL_MEMORY_NO_UPDATE_CHECK=off silently switching the check off, which is the opposite of what an operator typing that would expect. Nineteen tests in tests/unit/test_update_check_env_gate.py cover the accepted spellings, the rejected ones, the unset case, and that a test needing the real code path can still delete the variable. --- docs/reference/config.md | 3 +- src/surreal_memory/cli/update_check.py | 18 +++++- tests/conftest.py | 6 ++ tests/unit/test_update_check_env_gate.py | 79 ++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_update_check_env_gate.py diff --git a/docs/reference/config.md b/docs/reference/config.md index e00fdd2d..9f183e26 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -397,6 +397,7 @@ places, the environment wins. | `SURREAL_MEMORY_HOST` | `src/surreal_memory/utils/config.py` | | `SURREAL_MEMORY_HUB_URL` | `src/surreal_memory/unified_config.py` | | `SURREAL_MEMORY_INLINE_EMBED_TIMEOUT` | `src/surreal_memory/engine/encoder.py` | +| `SURREAL_MEMORY_NO_UPDATE_CHECK` | `src/surreal_memory/cli/update_check.py` | | `SURREAL_MEMORY_REASONING_ALLOW_REMOTE` | `src/surreal_memory/unified_config.py` | | `SURREAL_MEMORY_REASONING_EXTRA_DIRS` | `src/surreal_memory/unified_config.py` | | `SURREAL_MEMORY_REASONING_INJECTION` | `src/surreal_memory/unified_config.py` | @@ -414,4 +415,4 @@ places, the environment wins. --- -*Auto-generated by `scripts/gen_config_docs.py` from `unified_config.py` — 23 sections, 51 environment variables.* +*Auto-generated by `scripts/gen_config_docs.py` from `unified_config.py` — 23 sections, 52 environment variables.* diff --git a/src/surreal_memory/cli/update_check.py b/src/surreal_memory/cli/update_check.py index 6383f98d..7ffbea13 100755 --- a/src/surreal_memory/cli/update_check.py +++ b/src/surreal_memory/cli/update_check.py @@ -176,6 +176,22 @@ def _print_update_notice(current: str, latest: str) -> None: def run_update_check_background() -> None: - """Launch update check in a daemon thread. Non-blocking, fire-and-forget.""" + """Launch update check in a daemon thread. Non-blocking, fire-and-forget. + + Short-circuits when ``SURREAL_MEMORY_NO_UPDATE_CHECK`` is truthy under the + repo's canonical env-var convention (`1/true/yes/on`, case-insensitive — + the same ``_env_truthy`` used for ``SURREAL_MEMORY_EMBEDDING_ENABLED``, + ``SURREAL_MEMORY_SYNC_ENABLED``, ``SURREAL_MEMORY_REASONING_*``). + Anything else — including ``off``, ``no``, ``0``, ``false`` — means the + update check runs, so an env var that LOOKS like a switch never silently + disables the check the way an inverted convention would. The pytest + session sets it to ``"1"`` in ``tests/conftest.py::_isolated_home_dir`` + alongside the ``$HOME`` redirect, so the suite makes no PyPI calls even + on a runner with egress. + """ + from surreal_memory.unified_config import _env_truthy + + if _env_truthy(os.environ.get("SURREAL_MEMORY_NO_UPDATE_CHECK")): + return thread = threading.Thread(target=_check_and_notify, daemon=True) thread.start() diff --git a/tests/conftest.py b/tests/conftest.py index b5b65f40..c04a0d2a 100755 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -273,5 +273,11 @@ def _isolated_home_dir(tmp_path_factory: pytest.TempPathFactory) -> Generator[No (surrealmemory_dir / "config.toml").touch() mp = pytest.MonkeyPatch() mp.setenv("HOME", str(fake_home)) + # Companion to the $HOME redirect: `run_update_check_background()` in + # `cli/update_check.py` short-circuits on this env var, so the suite makes + # no PyPI calls even on a runner with egress. `#110`/`#121` closed the + # write side; this closes the network side. Individual tests that need to + # exercise the update-check code path itself can `monkeypatch.delenv` it. + mp.setenv("SURREAL_MEMORY_NO_UPDATE_CHECK", "1") yield mp.undo() diff --git a/tests/unit/test_update_check_env_gate.py b/tests/unit/test_update_check_env_gate.py new file mode 100644 index 00000000..2e7b5edf --- /dev/null +++ b/tests/unit/test_update_check_env_gate.py @@ -0,0 +1,79 @@ +"""Regression: `run_update_check_background()` respects a hermetic-mode env var. + +The daemon thread that `smem`'s CLI kicks off on almost every invocation calls +`urlopen(PYPI_URL, timeout=3)` against pypi.org. `#110`/`#121` closed the +write side of that path (`_isolated_home_dir` in `tests/conftest.py` +redirects `$HOME` for the whole session); this closes the network side, so a +pytest run inside a sandboxed or offline environment doesn't sit through per-test +3-second `OSError` swallows or make silent live PyPI requests. + +Contract: `SURREAL_MEMORY_NO_UPDATE_CHECK` follows the repo's canonical +env-var truthiness (`_env_truthy` in `unified_config.py`: `1/true/yes/on`, +case-insensitive, everything else false). A truthy value makes +`run_update_check_background()` a no-op *before* it starts the thread; any +other value — including `off`, `no`, `0`, `false`, arbitrary strings — +lets the check run. The env var is set to `"1"` session-wide in +`tests/conftest.py::_isolated_home_dir`, alongside the `$HOME` redirect — +same fixture, same rationale. +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from surreal_memory.cli import update_check + + +class TestUpdateCheckEnvGate: + """Guards the "no PyPI calls under pytest" contract.""" + + def test_env_var_short_circuits_before_thread_starts( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """With SURREAL_MEMORY_NO_UPDATE_CHECK=1, no daemon thread is created.""" + monkeypatch.setenv("SURREAL_MEMORY_NO_UPDATE_CHECK", "1") + with patch("surreal_memory.cli.update_check.threading.Thread") as thread_ctor: + update_check.run_update_check_background() + thread_ctor.assert_not_called() + + def test_env_var_missing_still_starts_thread(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Without the env var, the daemon thread does start — positive control + that the short-circuit is specifically the env var, not something else.""" + # `_isolated_home_dir` (session, autouse) sets this to "1" for the whole + # test session; explicitly clear it for this one test to prove the + # short-circuit is env-gated rather than a permanent no-op. + monkeypatch.delenv("SURREAL_MEMORY_NO_UPDATE_CHECK", raising=False) + with patch("surreal_memory.cli.update_check.threading.Thread") as thread_ctor: + update_check.run_update_check_background() + thread_ctor.assert_called_once() + # `daemon=True` in the call so a leaked thread never blocks interpreter shutdown. + _args, kwargs = thread_ctor.call_args + assert kwargs.get("daemon") is True + + @pytest.mark.parametrize( + "falsy", ["", "0", "false", "no", "FALSE", "No", "off", "OFF", "disabled", "anything"] + ) + def test_falsy_values_do_not_short_circuit( + self, monkeypatch: pytest.MonkeyPatch, falsy: str + ) -> None: + """Everything except `1/true/yes/on` counts as "not set" — including + `off`/`disabled`, which under an inverted convention would have + silently disabled the update check.""" + monkeypatch.setenv("SURREAL_MEMORY_NO_UPDATE_CHECK", falsy) + with patch("surreal_memory.cli.update_check.threading.Thread") as thread_ctor: + update_check.run_update_check_background() + thread_ctor.assert_called_once() + + @pytest.mark.parametrize("truthy", ["1", "yes", "true", "TRUE", "on", "YES", "On"]) + def test_truthy_values_short_circuit( + self, monkeypatch: pytest.MonkeyPatch, truthy: str + ) -> None: + """Exactly the repo's canonical truthy set, case-insensitive — the + same `_env_truthy` convention as `SURREAL_MEMORY_EMBEDDING_ENABLED` + and friends.""" + monkeypatch.setenv("SURREAL_MEMORY_NO_UPDATE_CHECK", truthy) + with patch("surreal_memory.cli.update_check.threading.Thread") as thread_ctor: + update_check.run_update_check_background() + thread_ctor.assert_not_called()