fix(hooks): real python-resolution for .sh hooks, with MEMPAL_PYTHON override#833
Open
fix(hooks): real python-resolution for .sh hooks, with MEMPAL_PYTHON override#833
Conversation
The legacy ``.sh`` hook scripts (``mempal_save_hook.sh``,
``mempal_precompact_hook.sh``) call ``python3 -m mempalace mine …``
directly. On macOS GUI launches of Claude Code — ``open -a``,
Spotlight, the dock — the harness inherits ``PATH`` from launchd
(``/usr/bin:/bin:/usr/sbin:/sbin``), which typically does not contain
the Python where ``mempalace`` was installed. The hook then silently
fails deep in the background log where users never look.
The original drive-by fix (branch ``fix/hook-bugs``) replaced
``python3`` with ``$(command -v python3)``. Verified empirically that
those two resolve to the same binary — the "fix" was a no-op, and the
"nohup strips PATH" rationale in the commit message doesn't apply
(the hooks don't use nohup).
This PR delivers the fix the original branch intended:
1. **MEMPAL_PYTHON override** — users whose GUI-launch PATH lacks the
right interpreter can export ``MEMPAL_PYTHON=/path/to/venv/python``
and the hooks pick it up first. Resolution order:
$MEMPAL_PYTHON (if set and executable)
→ $(command -v python3)
→ bare ``python3``
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 rather than
crashing the Stop flow.
3. **Applied to all hook python calls** — not just the ingest line.
The JSON parse and transcript-count calls use the same resolution
so that ``MEMPAL_PYTHON`` consistently controls interpretation for
the whole script.
4. **README** — adds a "Known Limitations" entry documenting the
macOS GUI PATH behaviour and the ``MEMPAL_PYTHON`` workaround.
(The "session restart after install" note was already on develop.)
5. **Regression tests** in ``tests/test_hooks_shell.py``:
* ``MEMPAL_PYTHON`` override wins over PATH (proved via a
marker-emitting shim that proxies to the real interpreter).
* Non-executable ``MEMPAL_PYTHON`` falls back to PATH rather than
crashing on permission denied.
* Unset ``MEMPAL_PYTHON`` resolves via PATH.
* Both hooks exit 0 and log the warning when the resolved
interpreter can't import mempalace — Claude Code never sees a
non-zero exit from the auto-ingest branch.
Tests are skipped on Windows (POSIX-only bash scripts). 810/810 on
Linux; the 5 new tests take ~0.2s total.
``hooks_cli.py`` (the Python implementation invoked via
``mempalace hook run …``) already uses ``sys.executable`` and is
therefore trivially correct — no changes needed there.
Supersedes abandoned branch ``fix/hook-bugs``.
Co-Authored-By: MSL <232237854+milla-jovovich@users.noreply.github.com>
igorls
added
area/hooks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The legacy shell hooks (
mempal_save_hook.sh,mempal_precompact_hook.sh) callpython3 -m mempalace mine …directly. On macOS GUI launches of Claude Code,PATHcomes from launchd (/usr/bin:/bin:/usr/sbin:/sbin), which usually does not contain the Python where the user installedmempalace. 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
MEMPAL_PYTHONoverride. Users whose GUI-launch PATH lacks the right interpreter canexport MEMPAL_PYTHON=/path/to/venv/pythonand hooks pick it up first. Resolution order:$MEMPAL_PYTHONif set and executable$(command -v python3)on PATHpython3as a last resortImport 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.logand skips the ingest — preserving the Claude Code contract of returning 0.Applied consistently. The JSON parse and transcript-count calls use the same resolution, so
MEMPAL_PYTHONconsistently controls the whole script.README
Known Limitations— documents the macOS GUIPATHbehavior and theMEMPAL_PYTHONworkaround.5 regression tests in
tests/test_hooks_shell.py:MEMPAL_PYTHONoverride wins over PATH (proven via marker-emitting shim).MEMPAL_PYTHONfalls back to PATH rather than crashing.MEMPAL_PYTHONresolves via PATH.Why the original fix wasn't enough
The abandoned
fix/hook-bugsbranch replacedpython3with\$(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 viamempalace hook run …) already usessys.executableand is trivially correct. No changes there.python3(most Linux and CLI-launched setups) need to do nothing — the fallback chain is unchanged for them.Co-Authored-By: MSL 232237854+milla-jovovich@users.noreply.github.com