Merge pull request #1021 from jphein/upstream-fix/silent-save-visibility
silent_guard default fixed to True per 2026-04-19 review. ImportError/AttributeError handling narrowed. CI all green.
This commit is contained in:
+51
-5
@@ -134,8 +134,35 @@ def _log(message: str):
|
|||||||
|
|
||||||
|
|
||||||
def _output(data: dict):
|
def _output(data: dict):
|
||||||
"""Print JSON to stdout with consistent formatting (pretty-printed)."""
|
"""Print JSON to stdout without importing modules that may redirect streams.
|
||||||
print(json.dumps(data, indent=2, ensure_ascii=False))
|
|
||||||
|
If mempalace.mcp_server is already loaded, reuse its saved real stdout fd.
|
||||||
|
Otherwise, write directly to fd 1 so hook responses still go to stdout even
|
||||||
|
if sys.stdout has been redirected elsewhere.
|
||||||
|
"""
|
||||||
|
payload = (json.dumps(data, indent=2, ensure_ascii=False) + "\n").encode("utf-8")
|
||||||
|
|
||||||
|
real_stdout_fd: int | None = None
|
||||||
|
mcp_mod = sys.modules.get("mempalace.mcp_server") or sys.modules.get(
|
||||||
|
f"{__package__}.mcp_server" if __package__ else "mcp_server"
|
||||||
|
)
|
||||||
|
if mcp_mod is not None:
|
||||||
|
real_stdout_fd = getattr(mcp_mod, "_REAL_STDOUT_FD", None)
|
||||||
|
|
||||||
|
fd = real_stdout_fd if real_stdout_fd is not None else 1
|
||||||
|
offset = 0
|
||||||
|
try:
|
||||||
|
while offset < len(payload):
|
||||||
|
try:
|
||||||
|
offset += os.write(fd, payload[offset:])
|
||||||
|
except InterruptedError:
|
||||||
|
continue
|
||||||
|
return
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
sys.stdout.buffer.write(payload)
|
||||||
|
sys.stdout.buffer.flush()
|
||||||
|
|
||||||
|
|
||||||
def _get_mine_dir(transcript_path: str = "") -> str:
|
def _get_mine_dir(transcript_path: str = "") -> str:
|
||||||
@@ -259,10 +286,29 @@ def hook_stop(data: dict, harness: str):
|
|||||||
stop_hook_active = parsed["stop_hook_active"]
|
stop_hook_active = parsed["stop_hook_active"]
|
||||||
transcript_path = parsed["transcript_path"]
|
transcript_path = parsed["transcript_path"]
|
||||||
|
|
||||||
# If already in a save cycle, let through (infinite-loop prevention)
|
# If already in a block-mode save cycle, let through (infinite-loop prevention).
|
||||||
|
# Silent mode saves directly without returning {"decision":"block"}, so there's
|
||||||
|
# no loop to prevent — and Claude Code's plugin dispatch sets this flag on every
|
||||||
|
# fire after the first, which would otherwise suppress all subsequent auto-saves.
|
||||||
if str(stop_hook_active).lower() in ("true", "1", "yes"):
|
if str(stop_hook_active).lower() in ("true", "1", "yes"):
|
||||||
_output({})
|
# Safe default: assume silent mode on any config-read failure so saves
|
||||||
return
|
# proceed rather than being silently dropped. Silent mode is the default
|
||||||
|
# (v3.3.0+), so if we can't read config, behave as if it's still on.
|
||||||
|
silent_guard = True
|
||||||
|
try:
|
||||||
|
from .config import MempalaceConfig
|
||||||
|
except ImportError as exc:
|
||||||
|
_log(
|
||||||
|
f"WARNING: could not import MempalaceConfig for stop guard: {exc}; defaulting to silent mode"
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
try:
|
||||||
|
silent_guard = MempalaceConfig().hook_silent_save
|
||||||
|
except AttributeError as exc:
|
||||||
|
_log(f"WARNING: could not read hook_silent_save: {exc}; defaulting to silent mode")
|
||||||
|
if not silent_guard:
|
||||||
|
_output({})
|
||||||
|
return
|
||||||
|
|
||||||
# Count human messages
|
# Count human messages
|
||||||
exchange_count = _count_human_messages(transcript_path)
|
exchange_count = _count_human_messages(transcript_path)
|
||||||
|
|||||||
@@ -220,6 +220,68 @@ def test_precompact_allows(tmp_path):
|
|||||||
# --- _log ---
|
# --- _log ---
|
||||||
|
|
||||||
|
|
||||||
|
def test_output_writes_to_real_stdout_fd_when_mcp_server_loaded():
|
||||||
|
"""_output() must reach fd 1 even when mcp_server has redirected sys.stdout."""
|
||||||
|
import types
|
||||||
|
|
||||||
|
fake_module = types.ModuleType("mempalace.mcp_server")
|
||||||
|
|
||||||
|
read_fd, write_fd = os.pipe()
|
||||||
|
try:
|
||||||
|
fake_module._REAL_STDOUT_FD = write_fd
|
||||||
|
with patch.dict("sys.modules", {"mempalace.mcp_server": fake_module}):
|
||||||
|
from mempalace.hooks_cli import _output
|
||||||
|
|
||||||
|
_output({"systemMessage": "test"})
|
||||||
|
|
||||||
|
os.close(write_fd)
|
||||||
|
written = b""
|
||||||
|
while True:
|
||||||
|
chunk = os.read(read_fd, 4096)
|
||||||
|
if not chunk:
|
||||||
|
break
|
||||||
|
written += chunk
|
||||||
|
finally:
|
||||||
|
os.close(read_fd)
|
||||||
|
|
||||||
|
data = json.loads(written.decode())
|
||||||
|
assert data["systemMessage"] == "test"
|
||||||
|
|
||||||
|
|
||||||
|
def test_output_falls_back_to_fd1_when_mcp_server_absent():
|
||||||
|
"""_output() writes to fd 1 directly when mcp_server is not loaded."""
|
||||||
|
read_fd, write_fd = os.pipe()
|
||||||
|
try:
|
||||||
|
orig_fd1 = os.dup(1)
|
||||||
|
os.dup2(write_fd, 1)
|
||||||
|
os.close(write_fd)
|
||||||
|
try:
|
||||||
|
modules_without_mcp = {
|
||||||
|
k: v for k, v in __import__("sys").modules.items() if "mcp_server" not in k
|
||||||
|
}
|
||||||
|
with patch.dict("sys.modules", modules_without_mcp, clear=True):
|
||||||
|
from mempalace.hooks_cli import _output
|
||||||
|
|
||||||
|
_output({"continue": True})
|
||||||
|
finally:
|
||||||
|
os.dup2(orig_fd1, 1)
|
||||||
|
os.close(orig_fd1)
|
||||||
|
except Exception:
|
||||||
|
os.close(read_fd)
|
||||||
|
raise
|
||||||
|
|
||||||
|
written = b""
|
||||||
|
while True:
|
||||||
|
chunk = os.read(read_fd, 4096)
|
||||||
|
if not chunk:
|
||||||
|
break
|
||||||
|
written += chunk
|
||||||
|
os.close(read_fd)
|
||||||
|
|
||||||
|
data = json.loads(written.decode())
|
||||||
|
assert data["continue"] is True
|
||||||
|
|
||||||
|
|
||||||
def test_log_writes_to_hook_log(tmp_path):
|
def test_log_writes_to_hook_log(tmp_path):
|
||||||
with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
|
with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
|
||||||
_log("test message")
|
_log("test message")
|
||||||
|
|||||||
Reference in New Issue
Block a user