Merge pull request #833 from MemPalace/fix/hooks-python-resolution
fix(hooks): real python-resolution for .sh hooks, with MEMPAL_PYTHON override
This commit is contained in:
Igor Lins e Silva
@@ -137,6 +137,19 @@ Example output:
|
||||
|
||||
**Hooks require session restart after install.** Claude Code loads hooks from `settings.json` at session start only. If you run `mempalace init` or manually edit hook config mid-session, the hooks won't fire until you restart Claude Code. This is a Claude Code limitation.
|
||||
|
||||
**`MEMPAL_PYTHON` override for the hook's internal Python calls.** The save hook parses its JSON input and counts transcript messages with `python3`. When the harness is launched from a GUI on macOS — `open -a`, Spotlight, the dock — its `PATH` is the minimal `/usr/bin:/bin:/usr/sbin:/sbin` inherited from `launchd`, not your shell PATH. If `python3` isn't on that PATH, those internal calls fail and the hook can't count exchanges.
|
||||
|
||||
Point the hook at any Python 3 interpreter to fix it:
|
||||
|
||||
```bash
|
||||
export MEMPAL_PYTHON="/usr/bin/python3" # system Python is fine
|
||||
export MEMPAL_PYTHON="$HOME/.venvs/mempalace/bin/python" # or your venv
|
||||
```
|
||||
|
||||
Resolution priority: `$MEMPAL_PYTHON` (if set and executable) → `$(command -v python3)` → bare `python3`. The interpreter only needs `json` and `sys` from the standard library — `mempalace` itself does not need to be installed in it.
|
||||
|
||||
Note: the `mempalace mine` auto-ingest runs via the `mempalace` CLI, so that command also needs to be on the hook's `PATH`. Installing with `pipx install mempalace` or `uv tool install mempalace` puts it on a stable global location; otherwise extend the hook environment's `PATH` to include your venv's `bin/`.
|
||||
|
||||
## Cost
|
||||
|
||||
**Zero extra tokens.** The hooks notify the AI that saves happened in the background — the AI doesn't need to write anything in the chat. All filing is handled automatically. Previous versions asked the AI to write diary entries and drawer content in the chat window, which cost ~$1/session in retransmitted tokens.
|
||||
|
||||
@@ -54,10 +54,17 @@ mkdir -p "$STATE_DIR"
|
||||
# Leave empty to skip auto-ingest (AI handles saving via the block reason).
|
||||
MEMPAL_DIR=""
|
||||
|
||||
# Resolve the Python interpreter. Same contract as mempal_save_hook.sh:
|
||||
# MEMPAL_PYTHON (explicit override) → $(command -v python3) → bare python3.
|
||||
MEMPAL_PYTHON_BIN="${MEMPAL_PYTHON:-}"
|
||||
if [ -z "$MEMPAL_PYTHON_BIN" ] || [ ! -x "$MEMPAL_PYTHON_BIN" ]; then
|
||||
MEMPAL_PYTHON_BIN="$(command -v python3 2>/dev/null || echo python3)"
|
||||
fi
|
||||
|
||||
# Read JSON input from stdin
|
||||
INPUT=$(cat)
|
||||
|
||||
SESSION_ID=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('session_id','unknown'))" 2>/dev/null)
|
||||
SESSION_ID=$(echo "$INPUT" | "$MEMPAL_PYTHON_BIN" -c "import sys,json; print(json.load(sys.stdin).get('session_id','unknown'))" 2>/dev/null)
|
||||
|
||||
echo "[$(date '+%H:%M:%S')] PRE-COMPACT triggered for session $SESSION_ID" >> "$STATE_DIR/hook.log"
|
||||
|
||||
|
||||
@@ -61,13 +61,30 @@ mkdir -p "$STATE_DIR"
|
||||
# Leave empty to skip auto-ingest (AI handles saving via the block reason).
|
||||
MEMPAL_DIR=""
|
||||
|
||||
# Resolve the Python interpreter the hook should use.
|
||||
#
|
||||
# Why this is nontrivial: GUI-launched Claude Code on macOS (or any harness
|
||||
# that doesn't inherit the user's shell PATH) may find a `python3` on PATH
|
||||
# that lacks mempalace — e.g. /usr/bin/python3 while the user installed
|
||||
# mempalace into a venv or pyenv. Users in that situation can point the
|
||||
# hook at the right interpreter by exporting MEMPAL_PYTHON.
|
||||
#
|
||||
# Resolution order (first hit wins):
|
||||
# 1. $MEMPAL_PYTHON — explicit user override (absolute path)
|
||||
# 2. $(command -v python3) — first python3 on the hook's PATH
|
||||
# 3. bare "python3" — last-resort fallback (hope the PATH has it)
|
||||
MEMPAL_PYTHON_BIN="${MEMPAL_PYTHON:-}"
|
||||
if [ -z "$MEMPAL_PYTHON_BIN" ] || [ ! -x "$MEMPAL_PYTHON_BIN" ]; then
|
||||
MEMPAL_PYTHON_BIN="$(command -v python3 2>/dev/null || echo python3)"
|
||||
fi
|
||||
|
||||
# Read JSON input from stdin
|
||||
INPUT=$(cat)
|
||||
|
||||
# Parse all fields in a single Python call (3x faster than separate invocations)
|
||||
# SECURITY: All values are sanitized before being interpolated into shell assignments.
|
||||
# stop_hook_active is coerced to a strict True/False to prevent command injection via eval.
|
||||
eval $(echo "$INPUT" | python3 -c "
|
||||
eval $(echo "$INPUT" | "$MEMPAL_PYTHON_BIN" -c "
|
||||
import sys, json, re
|
||||
data = json.load(sys.stdin)
|
||||
sid = data.get('session_id', 'unknown')
|
||||
@@ -95,7 +112,7 @@ fi
|
||||
# Count human messages in the JSONL transcript
|
||||
# SECURITY: Pass transcript path as sys.argv to avoid shell injection via crafted paths
|
||||
if [ -f "$TRANSCRIPT_PATH" ]; then
|
||||
EXCHANGE_COUNT=$(python3 - "$TRANSCRIPT_PATH" <<'PYEOF'
|
||||
EXCHANGE_COUNT=$("$MEMPAL_PYTHON_BIN" - "$TRANSCRIPT_PATH" <<'PYEOF'
|
||||
import json, sys
|
||||
count = 0
|
||||
with open(sys.argv[1]) as f:
|
||||
|
||||
@@ -0,0 +1,170 @@
|
||||
"""
|
||||
Integration tests for the legacy ``.sh`` hook scripts.
|
||||
|
||||
The shell hooks do their own Python resolution (unlike the Python
|
||||
``hooks_cli.py`` which uses ``sys.executable`` — trivially correct).
|
||||
GUI-launched harnesses on macOS provide a minimal PATH that often lacks
|
||||
the Python where ``mempalace`` is installed, so the shell path needs to:
|
||||
|
||||
1. honour ``$MEMPAL_PYTHON`` as an explicit user override;
|
||||
2. fall back to ``$(command -v python3)`` / bare ``python3``;
|
||||
3. *never* crash the hook when the resolved interpreter can't import
|
||||
mempalace — log and skip the auto-ingest instead, so Claude Code
|
||||
doesn't see a non-zero exit from its Stop hook.
|
||||
|
||||
These regressions matter because every failure mode they catch produced
|
||||
silent breakage in production — the user's hook appeared to "not fire"
|
||||
but was actually crashing deep in a PATH-resolution edge case.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import stat
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
SAVE_HOOK = REPO_ROOT / "hooks" / "mempal_save_hook.sh"
|
||||
PRECOMPACT_HOOK = REPO_ROOT / "hooks" / "mempal_precompact_hook.sh"
|
||||
|
||||
|
||||
pytestmark = pytest.mark.skipif(os.name == "nt", reason="bash hook scripts are POSIX-only")
|
||||
|
||||
|
||||
# ── helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def _write_fake_python(
|
||||
path: Path, *, can_import_mempalace: bool = False, marker_file: Path | None = None
|
||||
) -> Path:
|
||||
"""Create a python3 shim that proxies to the real interpreter so
|
||||
the hook's JSON-parsing calls still work, but fails ``-c 'import
|
||||
mempalace'`` / ``-m mempalace`` when ``can_import_mempalace`` is
|
||||
False.
|
||||
|
||||
Every invocation appends the shim name to ``marker_file`` so tests
|
||||
can prove which interpreter the hook invoked — using a file because
|
||||
the hook pipes some python calls to ``2>/dev/null``, so stderr
|
||||
markers are unreliable."""
|
||||
real_python = sys.executable
|
||||
marker = str(marker_file) if marker_file is not None else ""
|
||||
shim_src = f"""#!/bin/bash
|
||||
# Fake python3 shim: proxy to the real interpreter, drop a marker,
|
||||
# and simulate a missing mempalace install when configured that way.
|
||||
MARKER_FILE="{marker}"
|
||||
if [ -n "$MARKER_FILE" ]; then
|
||||
echo "{path.name}" >> "$MARKER_FILE"
|
||||
fi
|
||||
CAN_IMPORT={"1" if can_import_mempalace else "0"}
|
||||
# Simulate the "mempalace is not installed in this interpreter" case.
|
||||
if [ "$CAN_IMPORT" = "0" ]; then
|
||||
if [ "$1" = "-c" ] && echo "$2" | grep -q "import mempalace"; then
|
||||
exit 1
|
||||
fi
|
||||
if [ "$1" = "-m" ] && [ "$2" = "mempalace" ]; then
|
||||
exit 1
|
||||
fi
|
||||
fi
|
||||
# Everything else — JSON parsing, heredoc stdin, etc — delegate to real python.
|
||||
exec "{real_python}" "$@"
|
||||
"""
|
||||
path.write_text(shim_src)
|
||||
path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
|
||||
return path
|
||||
|
||||
|
||||
def _run_hook(
|
||||
script: Path,
|
||||
stdin_json: dict,
|
||||
*,
|
||||
env_overrides: dict | None = None,
|
||||
path_prefix: list[Path] | None = None,
|
||||
) -> subprocess.CompletedProcess:
|
||||
"""Invoke a shell hook with a minimal controlled environment."""
|
||||
env = {
|
||||
# Give the hook a clean slate — no inherited MEMPAL_* vars.
|
||||
"HOME": os.environ.get("HOME", "/tmp"),
|
||||
"PATH": os.environ.get("PATH", "/usr/bin:/bin"),
|
||||
}
|
||||
if path_prefix:
|
||||
env["PATH"] = os.pathsep.join(str(p) for p in path_prefix) + os.pathsep + env["PATH"]
|
||||
if env_overrides:
|
||||
env.update(env_overrides)
|
||||
return subprocess.run(
|
||||
["bash", str(script)],
|
||||
input=json.dumps(stdin_json),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
timeout=30,
|
||||
)
|
||||
|
||||
|
||||
# ── MEMPAL_PYTHON resolution contract ────────────────────────────────────
|
||||
|
||||
|
||||
class TestMempalPythonOverride:
|
||||
def test_explicit_override_wins_over_path(self, tmp_path):
|
||||
"""If MEMPAL_PYTHON is set and executable, the hook must use it
|
||||
in preference to whatever is on PATH."""
|
||||
marker = tmp_path / "markers.log"
|
||||
fake = _write_fake_python(
|
||||
tmp_path / "override_python",
|
||||
can_import_mempalace=True,
|
||||
marker_file=marker,
|
||||
)
|
||||
result = _run_hook(
|
||||
SAVE_HOOK,
|
||||
{"session_id": "abc", "stop_hook_active": False, "transcript_path": ""},
|
||||
env_overrides={"MEMPAL_PYTHON": str(fake), "HOME": str(tmp_path)},
|
||||
)
|
||||
assert (
|
||||
result.returncode == 0
|
||||
), f"hook exited non-zero: stderr={result.stderr!r} stdout={result.stdout!r}"
|
||||
invocations = marker.read_text().splitlines() if marker.exists() else []
|
||||
assert (
|
||||
"override_python" in invocations
|
||||
), f"MEMPAL_PYTHON override was not used. Marker log: {invocations!r}"
|
||||
|
||||
def test_ignores_override_when_not_executable(self, tmp_path):
|
||||
"""If MEMPAL_PYTHON is set but the file isn't executable, the
|
||||
hook must fall back to PATH rather than blow up with a
|
||||
'permission denied'."""
|
||||
bogus = tmp_path / "not_executable"
|
||||
bogus.write_text("# not a python")
|
||||
# Do NOT chmod +x — the hook should notice and skip.
|
||||
result = _run_hook(
|
||||
SAVE_HOOK,
|
||||
{"session_id": "abc", "stop_hook_active": False, "transcript_path": ""},
|
||||
env_overrides={"MEMPAL_PYTHON": str(bogus), "HOME": str(tmp_path)},
|
||||
)
|
||||
assert (
|
||||
result.returncode == 0
|
||||
), f"hook crashed on non-executable MEMPAL_PYTHON: {result.stderr!r}"
|
||||
|
||||
def test_falls_back_to_path_when_unset(self, tmp_path):
|
||||
"""With MEMPAL_PYTHON unset, the hook uses whatever ``python3``
|
||||
is found on PATH. Prove this by putting a marker-emitting shim
|
||||
first on PATH."""
|
||||
marker = tmp_path / "markers.log"
|
||||
fake = _write_fake_python(
|
||||
tmp_path / "python3",
|
||||
can_import_mempalace=True,
|
||||
marker_file=marker,
|
||||
)
|
||||
result = _run_hook(
|
||||
SAVE_HOOK,
|
||||
{"session_id": "abc", "stop_hook_active": False, "transcript_path": ""},
|
||||
env_overrides={"MEMPAL_PYTHON": "", "HOME": str(tmp_path)},
|
||||
path_prefix=[fake.parent],
|
||||
)
|
||||
assert result.returncode == 0
|
||||
invocations = marker.read_text().splitlines() if marker.exists() else []
|
||||
assert (
|
||||
"python3" in invocations
|
||||
), f"fallback-to-PATH did not use the shimmed python3. Marker log: {invocations!r}"
|
||||
Reference in New Issue
Block a user