Skip to content

Commit 1b00f93

Browse files
authored
Merge pull request #833 from MemPalace/fix/hooks-python-resolution
fix(hooks): real python-resolution for .sh hooks, with MEMPAL_PYTHON override
2 parents 5522d34 + 48eb627 commit 1b00f93

4 files changed

Lines changed: 210 additions & 3 deletions

File tree

hooks/README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,19 @@ Example output:
137137

138138
**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.
139139

140+
**`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.
141+
142+
Point the hook at any Python 3 interpreter to fix it:
143+
144+
```bash
145+
export MEMPAL_PYTHON="/usr/bin/python3" # system Python is fine
146+
export MEMPAL_PYTHON="$HOME/.venvs/mempalace/bin/python" # or your venv
147+
```
148+
149+
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.
150+
151+
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/`.
152+
140153
## Cost
141154

142155
**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.

hooks/mempal_precompact_hook.sh

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,17 @@ mkdir -p "$STATE_DIR"
5454
# Leave empty to skip auto-ingest (AI handles saving via the block reason).
5555
MEMPAL_DIR=""
5656

57+
# Resolve the Python interpreter. Same contract as mempal_save_hook.sh:
58+
# MEMPAL_PYTHON (explicit override) → $(command -v python3) → bare python3.
59+
MEMPAL_PYTHON_BIN="${MEMPAL_PYTHON:-}"
60+
if [ -z "$MEMPAL_PYTHON_BIN" ] || [ ! -x "$MEMPAL_PYTHON_BIN" ]; then
61+
MEMPAL_PYTHON_BIN="$(command -v python3 2>/dev/null || echo python3)"
62+
fi
63+
5764
# Read JSON input from stdin
5865
INPUT=$(cat)
5966

60-
SESSION_ID=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('session_id','unknown'))" 2>/dev/null)
67+
SESSION_ID=$(echo "$INPUT" | "$MEMPAL_PYTHON_BIN" -c "import sys,json; print(json.load(sys.stdin).get('session_id','unknown'))" 2>/dev/null)
6168

6269
echo "[$(date '+%H:%M:%S')] PRE-COMPACT triggered for session $SESSION_ID" >> "$STATE_DIR/hook.log"
6370

hooks/mempal_save_hook.sh

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,30 @@ mkdir -p "$STATE_DIR"
6161
# Leave empty to skip auto-ingest (AI handles saving via the block reason).
6262
MEMPAL_DIR=""
6363

64+
# Resolve the Python interpreter the hook should use.
65+
#
66+
# Why this is nontrivial: GUI-launched Claude Code on macOS (or any harness
67+
# that doesn't inherit the user's shell PATH) may find a `python3` on PATH
68+
# that lacks mempalace — e.g. /usr/bin/python3 while the user installed
69+
# mempalace into a venv or pyenv. Users in that situation can point the
70+
# hook at the right interpreter by exporting MEMPAL_PYTHON.
71+
#
72+
# Resolution order (first hit wins):
73+
# 1. $MEMPAL_PYTHON — explicit user override (absolute path)
74+
# 2. $(command -v python3) — first python3 on the hook's PATH
75+
# 3. bare "python3" — last-resort fallback (hope the PATH has it)
76+
MEMPAL_PYTHON_BIN="${MEMPAL_PYTHON:-}"
77+
if [ -z "$MEMPAL_PYTHON_BIN" ] || [ ! -x "$MEMPAL_PYTHON_BIN" ]; then
78+
MEMPAL_PYTHON_BIN="$(command -v python3 2>/dev/null || echo python3)"
79+
fi
80+
6481
# Read JSON input from stdin
6582
INPUT=$(cat)
6683

6784
# Parse all fields in a single Python call (3x faster than separate invocations)
6885
# SECURITY: All values are sanitized before being interpolated into shell assignments.
6986
# stop_hook_active is coerced to a strict True/False to prevent command injection via eval.
70-
eval $(echo "$INPUT" | python3 -c "
87+
eval $(echo "$INPUT" | "$MEMPAL_PYTHON_BIN" -c "
7188
import sys, json, re
7289
data = json.load(sys.stdin)
7390
sid = data.get('session_id', 'unknown')
@@ -95,7 +112,7 @@ fi
95112
# Count human messages in the JSONL transcript
96113
# SECURITY: Pass transcript path as sys.argv to avoid shell injection via crafted paths
97114
if [ -f "$TRANSCRIPT_PATH" ]; then
98-
EXCHANGE_COUNT=$(python3 - "$TRANSCRIPT_PATH" <<'PYEOF'
115+
EXCHANGE_COUNT=$("$MEMPAL_PYTHON_BIN" - "$TRANSCRIPT_PATH" <<'PYEOF'
99116
import json, sys
100117
count = 0
101118
with open(sys.argv[1]) as f:

tests/test_hooks_shell.py

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
"""
2+
Integration tests for the legacy ``.sh`` hook scripts.
3+
4+
The shell hooks do their own Python resolution (unlike the Python
5+
``hooks_cli.py`` which uses ``sys.executable`` — trivially correct).
6+
GUI-launched harnesses on macOS provide a minimal PATH that often lacks
7+
the Python where ``mempalace`` is installed, so the shell path needs to:
8+
9+
1. honour ``$MEMPAL_PYTHON`` as an explicit user override;
10+
2. fall back to ``$(command -v python3)`` / bare ``python3``;
11+
3. *never* crash the hook when the resolved interpreter can't import
12+
mempalace — log and skip the auto-ingest instead, so Claude Code
13+
doesn't see a non-zero exit from its Stop hook.
14+
15+
These regressions matter because every failure mode they catch produced
16+
silent breakage in production — the user's hook appeared to "not fire"
17+
but was actually crashing deep in a PATH-resolution edge case.
18+
"""
19+
20+
from __future__ import annotations
21+
22+
import json
23+
import os
24+
import stat
25+
import subprocess
26+
import sys
27+
from pathlib import Path
28+
29+
import pytest
30+
31+
REPO_ROOT = Path(__file__).resolve().parent.parent
32+
SAVE_HOOK = REPO_ROOT / "hooks" / "mempal_save_hook.sh"
33+
PRECOMPACT_HOOK = REPO_ROOT / "hooks" / "mempal_precompact_hook.sh"
34+
35+
36+
pytestmark = pytest.mark.skipif(os.name == "nt", reason="bash hook scripts are POSIX-only")
37+
38+
39+
# ── helpers ───────────────────────────────────────────────────────────────
40+
41+
42+
def _write_fake_python(
43+
path: Path, *, can_import_mempalace: bool = False, marker_file: Path | None = None
44+
) -> Path:
45+
"""Create a python3 shim that proxies to the real interpreter so
46+
the hook's JSON-parsing calls still work, but fails ``-c 'import
47+
mempalace'`` / ``-m mempalace`` when ``can_import_mempalace`` is
48+
False.
49+
50+
Every invocation appends the shim name to ``marker_file`` so tests
51+
can prove which interpreter the hook invoked — using a file because
52+
the hook pipes some python calls to ``2>/dev/null``, so stderr
53+
markers are unreliable."""
54+
real_python = sys.executable
55+
marker = str(marker_file) if marker_file is not None else ""
56+
shim_src = f"""#!/bin/bash
57+
# Fake python3 shim: proxy to the real interpreter, drop a marker,
58+
# and simulate a missing mempalace install when configured that way.
59+
MARKER_FILE="{marker}"
60+
if [ -n "$MARKER_FILE" ]; then
61+
echo "{path.name}" >> "$MARKER_FILE"
62+
fi
63+
CAN_IMPORT={"1" if can_import_mempalace else "0"}
64+
# Simulate the "mempalace is not installed in this interpreter" case.
65+
if [ "$CAN_IMPORT" = "0" ]; then
66+
if [ "$1" = "-c" ] && echo "$2" | grep -q "import mempalace"; then
67+
exit 1
68+
fi
69+
if [ "$1" = "-m" ] && [ "$2" = "mempalace" ]; then
70+
exit 1
71+
fi
72+
fi
73+
# Everything else — JSON parsing, heredoc stdin, etc — delegate to real python.
74+
exec "{real_python}" "$@"
75+
"""
76+
path.write_text(shim_src)
77+
path.chmod(path.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
78+
return path
79+
80+
81+
def _run_hook(
82+
script: Path,
83+
stdin_json: dict,
84+
*,
85+
env_overrides: dict | None = None,
86+
path_prefix: list[Path] | None = None,
87+
) -> subprocess.CompletedProcess:
88+
"""Invoke a shell hook with a minimal controlled environment."""
89+
env = {
90+
# Give the hook a clean slate — no inherited MEMPAL_* vars.
91+
"HOME": os.environ.get("HOME", "/tmp"),
92+
"PATH": os.environ.get("PATH", "/usr/bin:/bin"),
93+
}
94+
if path_prefix:
95+
env["PATH"] = os.pathsep.join(str(p) for p in path_prefix) + os.pathsep + env["PATH"]
96+
if env_overrides:
97+
env.update(env_overrides)
98+
return subprocess.run(
99+
["bash", str(script)],
100+
input=json.dumps(stdin_json),
101+
capture_output=True,
102+
text=True,
103+
env=env,
104+
timeout=30,
105+
)
106+
107+
108+
# ── MEMPAL_PYTHON resolution contract ────────────────────────────────────
109+
110+
111+
class TestMempalPythonOverride:
112+
def test_explicit_override_wins_over_path(self, tmp_path):
113+
"""If MEMPAL_PYTHON is set and executable, the hook must use it
114+
in preference to whatever is on PATH."""
115+
marker = tmp_path / "markers.log"
116+
fake = _write_fake_python(
117+
tmp_path / "override_python",
118+
can_import_mempalace=True,
119+
marker_file=marker,
120+
)
121+
result = _run_hook(
122+
SAVE_HOOK,
123+
{"session_id": "abc", "stop_hook_active": False, "transcript_path": ""},
124+
env_overrides={"MEMPAL_PYTHON": str(fake), "HOME": str(tmp_path)},
125+
)
126+
assert (
127+
result.returncode == 0
128+
), f"hook exited non-zero: stderr={result.stderr!r} stdout={result.stdout!r}"
129+
invocations = marker.read_text().splitlines() if marker.exists() else []
130+
assert (
131+
"override_python" in invocations
132+
), f"MEMPAL_PYTHON override was not used. Marker log: {invocations!r}"
133+
134+
def test_ignores_override_when_not_executable(self, tmp_path):
135+
"""If MEMPAL_PYTHON is set but the file isn't executable, the
136+
hook must fall back to PATH rather than blow up with a
137+
'permission denied'."""
138+
bogus = tmp_path / "not_executable"
139+
bogus.write_text("# not a python")
140+
# Do NOT chmod +x — the hook should notice and skip.
141+
result = _run_hook(
142+
SAVE_HOOK,
143+
{"session_id": "abc", "stop_hook_active": False, "transcript_path": ""},
144+
env_overrides={"MEMPAL_PYTHON": str(bogus), "HOME": str(tmp_path)},
145+
)
146+
assert (
147+
result.returncode == 0
148+
), f"hook crashed on non-executable MEMPAL_PYTHON: {result.stderr!r}"
149+
150+
def test_falls_back_to_path_when_unset(self, tmp_path):
151+
"""With MEMPAL_PYTHON unset, the hook uses whatever ``python3``
152+
is found on PATH. Prove this by putting a marker-emitting shim
153+
first on PATH."""
154+
marker = tmp_path / "markers.log"
155+
fake = _write_fake_python(
156+
tmp_path / "python3",
157+
can_import_mempalace=True,
158+
marker_file=marker,
159+
)
160+
result = _run_hook(
161+
SAVE_HOOK,
162+
{"session_id": "abc", "stop_hook_active": False, "transcript_path": ""},
163+
env_overrides={"MEMPAL_PYTHON": "", "HOME": str(tmp_path)},
164+
path_prefix=[fake.parent],
165+
)
166+
assert result.returncode == 0
167+
invocations = marker.read_text().splitlines() if marker.exists() else []
168+
assert (
169+
"python3" in invocations
170+
), f"fallback-to-PATH did not use the shimmed python3. Marker log: {invocations!r}"

0 commit comments

Comments
 (0)