diff --git a/claude_sdk_executor.py b/claude_sdk_executor.py index c2a504e..1219c41 100644 --- a/claude_sdk_executor.py +++ b/claude_sdk_executor.py @@ -147,16 +147,26 @@ def _mark_sdk_wedged(reason: str) -> None: if _sdk_wedged_reason is None: _sdk_wedged_reason = reason logger.error("SDK wedge detected: %s — workspace will report degraded until a successful query clears it", reason) + # Catch is narrowed to import errors: a SIGNATURE drift + # (mark_wedged renamed/removed) must surface so the smoke gate + # + heartbeat aren't silently blind. The runtime's structural + # snapshot test (molecule-core task #169) catches the rename + # at PR-time. Older runtimes that don't ship runtime_wedge at + # all hit ImportError here and silently no-op the mirror — + # the local sticky flag still gates is_wedged() inside this + # module so internal callers (retry loop, cancel handler) + # keep working. try: from molecule_runtime.runtime_wedge import mark_wedged as _mark_runtime_wedged - except Exception: + except (ImportError, ModuleNotFoundError): return try: _mark_runtime_wedged(reason) except Exception: - # Mirror is best-effort — a runtime_wedge regression - # (signature change, internal raise) must not silently - # suppress the local wedge state. + # Mirror call (not import) is still best-effort — a + # runtime_wedge internal raise must not silently suppress + # the local wedge state. Logged loudly so the regression + # is at least visible in the executor log. logger.exception("runtime_wedge.mark_wedged mirror failed — local SDK wedge flag is still set") @@ -177,9 +187,10 @@ def _clear_sdk_wedge_on_success() -> None: if _sdk_wedged_reason is not None: logger.info("SDK wedge cleared after successful query — workspace will recover to online on next heartbeat") _sdk_wedged_reason = None + # Same import-narrowing rationale as _mark_sdk_wedged above. try: from molecule_runtime.runtime_wedge import clear_wedge as _clear_runtime_wedge - except Exception: + except (ImportError, ModuleNotFoundError): return try: _clear_runtime_wedge() diff --git a/tests/test_runtime_wedge_mirror.py b/tests/test_runtime_wedge_mirror.py index 4c714bc..fa656bc 100644 --- a/tests/test_runtime_wedge_mirror.py +++ b/tests/test_runtime_wedge_mirror.py @@ -228,3 +228,100 @@ def test_mirror_swallows_runtime_wedge_import_error(): # Clear path also swallows. mod._clear_sdk_wedge_on_success() assert mod.is_wedged() is False + + +# ─── Mirror-call-failure injection (review follow-up) ────────────────── +# +# The recorder above never raises, so the inner `try` arm around +# `_mark_runtime_wedged(reason)` (and the symmetric clear) wasn't +# pinned by the original mirror tests. Inject a recorder whose +# call-side raises so the catch arm is exercised: the mirror failure +# must be logged but must NOT suppress the local sticky flag. + + +def _install_runtime_wedge_raising_recorder() -> dict: + """Replace molecule_runtime.runtime_wedge with a recorder whose + mark_wedged + clear_wedge implementations RAISE on call (not on + import). Captures the call-attempt count so the test can verify + the catch arm fired without leaking the exception. Returns the + recorder dict (mark_attempts, clear_attempts).""" + rec = {"mark_attempts": 0, "clear_attempts": 0} + mod = types.ModuleType("molecule_runtime.runtime_wedge") + + def _mark(_reason: str) -> None: + rec["mark_attempts"] += 1 + raise RuntimeError("simulated runtime_wedge.mark_wedged internal raise") + + def _clear() -> None: + rec["clear_attempts"] += 1 + raise RuntimeError("simulated runtime_wedge.clear_wedge internal raise") + + mod.mark_wedged = _mark + mod.clear_wedge = _clear + sys.modules["molecule_runtime.runtime_wedge"] = mod + return rec + + +def test_mark_sdk_wedged_swallows_mirror_call_exception(caplog): + """If runtime_wedge.mark_wedged itself raises (signature is fine, + body has a bug), the caller in claude_sdk_executor must log AND + keep the local sticky flag set. Otherwise an internal regression + in runtime_wedge would silently make this workspace appear healthy + while every chat actually hangs. + """ + import logging + rec = _install_runtime_wedge_raising_recorder() + mod = _load_executor() + mod._reset_sdk_wedge_for_test() + + with caplog.at_level(logging.ERROR, logger="claude_sdk_executor"): + mod._mark_sdk_wedged("local-and-mirror reason") + + assert rec["mark_attempts"] == 1, ( + "executor never called runtime_wedge.mark_wedged — the inner " + "try block was skipped or short-circuited" + ) + assert mod.is_wedged() is True, ( + "mirror-call exception suppressed the local sticky flag — " + "violates the 'mirror is best-effort, local is source of truth' " + "contract" + ) + assert mod.wedge_reason() == "local-and-mirror reason" + # Loud log line is the only operator-visible signal that the mirror + # silently failed — pin its presence so a future logger.exception → + # logger.debug downgrade can't sneak through. + assert any( + "runtime_wedge.mark_wedged mirror failed" in r.message + for r in caplog.records + ), "mirror-call failure was not logged at ERROR — operator can't see the regression" + + +def test_clear_sdk_wedge_on_success_swallows_mirror_call_exception(caplog): + """Symmetric to the mark test: a runtime_wedge.clear_wedge bug + must not leave the local flag stuck-on (which would make + auto-recovery silently broken even though the SDK started working + again).""" + import logging + rec = _install_runtime_wedge_raising_recorder() + mod = _load_executor() + mod._reset_sdk_wedge_for_test() + mod._mark_sdk_wedged("transient") + # Mark also raised but local flag is set — that's the precondition. + assert mod.is_wedged() is True + rec["mark_attempts"] = 0 # only count the clear attempt below + + with caplog.at_level(logging.ERROR, logger="claude_sdk_executor"): + mod._clear_sdk_wedge_on_success() + + assert rec["clear_attempts"] == 1, ( + "executor never called runtime_wedge.clear_wedge — inner try " + "block was skipped" + ) + assert mod.is_wedged() is False, ( + "mirror clear-call exception left the local sticky flag set — " + "auto-recovery is silently broken" + ) + assert any( + "runtime_wedge.clear_wedge mirror failed" in r.message + for r in caplog.records + ), "clear mirror-call failure was not logged at ERROR"