Merge pull request #30 from Molecule-AI/chore/runtime-wedge-mirror-followups

chore(executor): runtime_wedge mirror follow-ups from PR #29 review

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()

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"