chore(executor): runtime_wedge mirror follow-ups from PR #29 review
Two review nits: 1. Narrow the import-arm catch in _mark_sdk_wedged and _clear_sdk_wedge_on_success to (ImportError, ModuleNotFoundError). The bare `except Exception:` would have masked an AttributeError / TypeError from a runtime_wedge API rename — silently degrading the mirror to "no-op" and making heartbeat + the smoke gate (#131) blind to claude-code wedges. The structural snapshot test in molecule-core (task #169) catches the rename at PR-time. Older runtimes that don't ship runtime_wedge at all still hit ImportError and silently no-op — the local sticky flag still gates is_wedged() inside this module so internal callers keep working. 2. Add mirror-CALL-failure injection tests. The recorder used by the original tests never raised, so the inner try around _mark_runtime_wedged(reason) (and the symmetric clear) wasn't pinned. New tests inject a recorder whose mark/clear raise on call, then assert: (a) the call attempt was recorded, (b) the local sticky flag stayed correct, (c) the failure was logged at ERROR. Pins both the contract ("mirror is best-effort, local is source of truth") AND the operator-visible signal (an ERROR log line is the only way to see a silent mirror regression). Regression-injection-checked: removing the call-side try arm makes both new tests fail with clear messages. Tests: 7 in test_runtime_wedge_mirror.py, 45 across the whole tests/ tree.
This commit is contained in:
parent
2dcedc14d3
commit
02e4520cf3
@ -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()
|
||||
|
||||
@ -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"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user