From 661eec2659bc0a7e517a6d0d8132a9ce4f2e9629 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 30 Apr 2026 21:31:08 -0700 Subject: [PATCH] chore(smoke-mode): harden module-load + drop dead except clause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from the #2275 Phase 1 self-review: 1. `_SMOKE_TIMEOUT_SECS = float(os.environ.get(...))` was evaluated at module load. main.py imports smoke_mode unconditionally — before the is_smoke_mode() check — so a malformed MOLECULE_SMOKE_TIMEOUT_SECS env value would SystemExit every workspace boot, not just smoke runs. Wrapped in try/except with a 5.0 fallback. Probability of a typo'd env var hitting production is low (it's a CI-only knob), but the footgun is removed entirely. Regression test reloads the module under a malformed env value. 2. `_real_a2a_sdk_available()` caught (ImportError, AttributeError). `from X import Y` raises ImportError when Y is missing on X — never AttributeError. Dropped the unreachable branch. No behavior change for the happy path. Co-Authored-By: Claude Opus 4.7 (1M context) --- workspace/smoke_mode.py | 8 +++++++- workspace/tests/test_smoke_mode.py | 23 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/workspace/smoke_mode.py b/workspace/smoke_mode.py index 773e0cbe..79399946 100644 --- a/workspace/smoke_mode.py +++ b/workspace/smoke_mode.py @@ -36,7 +36,13 @@ from typing import Any logger = logging.getLogger(__name__) -_SMOKE_TIMEOUT_SECS = float(os.environ.get("MOLECULE_SMOKE_TIMEOUT_SECS", "5.0")) +# Don't crash production boot if MOLECULE_SMOKE_TIMEOUT_SECS is malformed — +# main.py imports smoke_mode unconditionally (before the is_smoke_mode() +# check), so a typo'd value would otherwise SystemExit every workspace. +try: + _SMOKE_TIMEOUT_SECS = float(os.environ.get("MOLECULE_SMOKE_TIMEOUT_SECS", "5.0")) +except ValueError: + _SMOKE_TIMEOUT_SECS = 5.0 def is_smoke_mode() -> bool: diff --git a/workspace/tests/test_smoke_mode.py b/workspace/tests/test_smoke_mode.py index 10edbe30..9721024f 100644 --- a/workspace/tests/test_smoke_mode.py +++ b/workspace/tests/test_smoke_mode.py @@ -33,7 +33,7 @@ def _real_a2a_sdk_available() -> bool: from a2a.server.context import ServerCallContext # noqa: F401 from a2a.types import SendMessageRequest # noqa: F401 return True - except (ImportError, AttributeError): + except ImportError: return False @@ -57,6 +57,27 @@ def test_is_smoke_mode_unset(monkeypatch: pytest.MonkeyPatch): assert smoke_mode.is_smoke_mode() is False +# ─── _SMOKE_TIMEOUT_SECS bad-env-var resilience ──────────────────────── + + +def test_smoke_timeout_falls_back_when_env_value_is_malformed( + monkeypatch: pytest.MonkeyPatch, +): + """A typo'd MOLECULE_SMOKE_TIMEOUT_SECS must not crash production + boot. main.py imports smoke_mode unconditionally — before the + is_smoke_mode() check — so float()-at-module-load would SystemExit + every workspace if the env value were bad.""" + import importlib + monkeypatch.setenv("MOLECULE_SMOKE_TIMEOUT_SECS", "not-a-float") + reloaded = importlib.reload(smoke_mode) + try: + assert reloaded._SMOKE_TIMEOUT_SECS == 5.0 + finally: + # Restore module to clean default for other tests. + monkeypatch.delenv("MOLECULE_SMOKE_TIMEOUT_SECS", raising=False) + importlib.reload(smoke_mode) + + # ─── _build_stub_context (real-SDK-only) ───────────────────────────────