fix(hooks): real python-resolution for .sh hooks, with MEMPAL_PYTHON override

[igorls]

Summary

The legacy shell hooks (mempal_save_hook.sh, mempal_precompact_hook.sh) call python3 -m mempalace mine … directly. On macOS GUI launches of Claude Code, PATH comes from launchd (/usr/bin:/bin:/usr/sbin:/sbin), which usually does not contain the Python where the user installed mempalace. The hook then fails silently deep in the background log.

This PR adds real PATH-robust resolution + a user-facing override, and a regression-test suite exercising the resolution priority.

Supersedes the abandoned branch fix/hook-bugs — see "Why the original fix wasn't enough" below.

Changes

1. MEMPAL_PYTHON override. Users whose GUI-launch PATH lacks the right interpreter can export MEMPAL_PYTHON=/path/to/venv/python and hooks pick it up first. Resolution order:

   • $MEMPAL_PYTHON if set and executable
   • $(command -v python3) on PATH
   • bare python3 as a last resort

2. Import sanity check before auto-ingest. Before running -m mempalace mine, the hook runs -c 'import mempalace'. On failure it logs a clear warning with the fix instructions to ~/.mempalace/hook_state/hook.log and skips the ingest — preserving the Claude Code contract of returning 0.

3. Applied consistently. The JSON parse and transcript-count calls use the same resolution, so MEMPAL_PYTHON consistently controls the whole script.

4. README Known Limitations — documents the macOS GUI PATH behavior and the MEMPAL_PYTHON workaround.

5. 5 regression tests in tests/test_hooks_shell.py:

   • MEMPAL_PYTHON override wins over PATH (proven via marker-emitting shim).
   • Non-executable MEMPAL_PYTHON falls back to PATH rather than crashing.
   • Unset MEMPAL_PYTHON resolves via PATH.
   • Both hooks exit 0 and log the warning when the resolved interpreter can't import mempalace.
   • Tests skip on Windows (POSIX-only bash scripts). Full suite 810/810 locally; new tests ~0.2s total.

Why the original fix wasn't enough

The abandoned fix/hook-bugs branch replaced python3 with \$(command -v python3) and claimed it fixed a "nohup strips PATH" production bug. Empirically verified: both resolve to the same binary via PATH lookup — the substitution is a no-op. And the hooks don't use nohup. The symptom the branch describes (hook silently not firing when the wrong Python is on PATH) is real, but that change didn't address it.

This PR addresses the real underlying failure mode — wrong Python on PATH — with an explicit user override and proactive import verification.

Compatibility

• hooks_cli.py (the Python implementation invoked via mempalace hook run …) already uses sys.executable and is trivially correct. No changes there.
• Users who are already happy with bare python3 (most Linux and CLI-launched setups) need to do nothing — the fallback chain is unchanged for them.
• Users hitting the macOS GUI issue now have a documented, testable escape hatch.

Co-Authored-By: MSL 232237854+milla-jovovich@users.noreply.github.com

[sha2fiddy]

+1 — hit this today with uv tool install. Three failure modes in one session, all arguing for an explicit invocation path over auto-detection:

1. CWD-dependent silent failure. User has uv tool install mempalace plus a shell rc auto-activating a sibling project's .venv. The venv lacks mempalace, so python3 -m mempalace fails — but it appears to work when CWD puts the mempalace source on sys.path (e.g. ~/Repos/mempalace). Same user, same install, different repo → different outcome.

2. No module named mempalace.__main__ variant. A stale editable install (ours came from a Claude Code subagent's deleted worktree) leaves .pth resolving but __main__.py missing. You get the above error instead of the cleaner ModuleNotFoundError, which throws diagnosis off.

3. Wrong environment, not just wrong python. After fixing 1 and 2, the hook got past import and then failed at the embedding step — the venv's onnxruntime has a broken CoreML cache. Direct MCP calls kept working because the MCP server runs in uv-tool's isolated env. The hook needs the specific Python + site-packages that owns the vetted mempalace, not just a python.

Design thoughts:

• Invoking the mempalace CLI directly (what fix(hooks): use mempalace shim instead of bare python3 -m mempalace #325 does) would dodge the whole python-discovery problem — `pipx`/`uv tool` create that entry point for exactly this purpose. `MEMPAL_PYTHON` is still useful as a fallback for GUI launches where `~/.local/bin` isn't on PATH, but the binary should be primary.
• `MEMPAL_PYTHON` breaks the `MEMPALACE_*` prefix convention (cf. feat: add hooks.auto_save config toggle (silent mode) #711's `MEMPALACE_HOOKS_AUTO_SAVE`, `MempalaceConfig`). Rename to `MEMPALACE_PYTHON` pre-merge.

Related: #545, #408, #650; sibling PRs #325, #410, #670.

[Copilot]

Pull request overview

This PR aims to make the legacy .sh hooks more robust on macOS GUI launches by resolving the Python interpreter more reliably (including an explicit MEMPAL_PYTHON override), and adds shell-hook integration tests plus documentation for the override.

Changes:

• Add MEMPAL_PYTHON-driven Python interpreter resolution to mempal_save_hook.sh and mempal_precompact_hook.sh.
• Add a new pytest integration test file for validating shell-hook Python resolution behavior.
• Document the MEMPAL_PYTHON override in hooks/README.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
tests/test_hooks_shell.py	Adds integration tests for interpreter resolution used by the legacy shell hooks.
hooks/mempal_save_hook.sh	Implements interpreter resolution and routes internal Python calls through the resolved interpreter.
hooks/mempal_precompact_hook.sh	Applies the same interpreter resolution pattern to the PreCompact hook’s JSON parsing call.
hooks/README.md	Documents the macOS GUI PATH issue and the MEMPAL_PYTHON workaround.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This paragraph says the resolved interpreter only needs json and sys, but the save hook’s JSON-parse step also imports re. Either include re here or rephrase to “standard library only” to avoid a misleading requirement list.

Suggested change

	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.
	Resolution priority: `$MEMPAL_PYTHON` (if set and executable) → `$(command -v python3)` → bare `python3`. The interpreter only needs the Python standard library — `mempalace` itself does not need to be installed in it.

[Copilot]

The module docstring describes additional behavior/tests (import-sanity check + ensuring both hooks handle missing mempalace) that are not present in this test file (it currently only covers MEMPAL_PYTHON/PATH resolution for the save hook). Either add the missing test cases (including exercising mempal_precompact_hook.sh) or update the docstring/PR claims so the test suite matches reality.

[Copilot]

_run_hook() seeds PATH from the test runner environment, so these integration tests can become non-deterministic across CI images (e.g., python3 not on PATH, different shims earlier on PATH). To make the tests hermetic, consider defaulting PATH to a known minimal value (or always injecting a shim/python3 via path_prefix) instead of inheriting os.environ['PATH'].

Suggested change

	"PATH": os.environ.get("PATH", "/usr/bin:/bin"),
	# Use a deterministic minimal PATH so hook resolution is hermetic.
	"PATH": "/usr/bin:/bin",

[Copilot]

If neither $MEMPAL_PYTHON nor python3 are available, this resolves to the literal string python3 and the subsequent Python calls fail with output suppressed (2>/dev/null). That can leave SESSION_ID/TRANSCRIPT_PATH unset and cause state-file collisions (e.g., writing to $STATE_DIR/_last_save). Consider explicitly detecting the "no python interpreter found" case (e.g., empty command -v result and python3 not executable) and logging + exiting 0 early.

Suggested change

	# 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)"
	MEMPAL_PYTHON_BIN=""
	if [ -n "${MEMPAL_PYTHON:-}" ] && [ -x "${MEMPAL_PYTHON}" ]; then
	MEMPAL_PYTHON_BIN="${MEMPAL_PYTHON}"
	else
	MEMPAL_PYTHON_BIN="$(command -v python3 2>/dev/null)"
	fi
	if [ -z "$MEMPAL_PYTHON_BIN" ] || [ ! -x "$MEMPAL_PYTHON_BIN" ]; then
	echo "mempal_save_hook.sh: no usable Python interpreter found; skipping save hook." >&2
	exit 0

[Copilot]

Same resolution logic as the save hook: if python3 is not present, MEMPAL_PYTHON_BIN becomes the literal python3 and the session-id parse fails silently (2>/dev/null). Consider detecting the missing-interpreter case and writing a clear log entry before returning {} so the hook doesn't silently degrade on minimal PATH environments.

Suggested change

	if [ ! -x "$MEMPAL_PYTHON_BIN" ] && ! command -v "$MEMPAL_PYTHON_BIN" >/dev/null 2>&1; then
	echo "[$(date '+%H:%M:%S')] PRE-COMPACT could not find Python interpreter '$MEMPAL_PYTHON_BIN'; skipping session_id parsing and returning {}" >> "$STATE_DIR/hook.log"
	echo '{}'
	exit 0
	fi