fix(plugin): restore venv-aware Python resolution for hooks + MCP by jphein · Pull Request #1115 · MemPalace/mempalace

jphein · 2026-04-22T20:12:52Z

Summary

Three recent .claude-plugin/ changes regressed hook and MCP invocation for installs that depend on a local venv:

5fe0c1c — mempal-stop-hook.sh → PATH-only (command -v mempalace / bare python3 -m mempalace)
be9214a — mempal-precompact-hook.sh → same regression
9f5b8f5 — .mcp.json → bare "mempalace-mcp" command

All three assume a system-wide install via pipx/uv/pip-global — fine for that path, but they break:

Editable dev installs (pip install -e . inside a repo venv). mempalace / mempalace-mcp and the module only live in the repo venv, not on PATH. The stop/precompact hooks fall through all three checks and print:
```
MemPalace hook error: could not find a runnable mempalace command or module
```
MCP fails to spawn because bare mempalace-mcp isn't on PATH.
Claude Code plugin installs that create a per-plugin venv at ${CLAUDE_PLUGIN_ROOT}/venv/. Neither the hooks nor .mcp.json look there, so anything that isn't also system-installed breaks.

Fix

Restore the three-tier resolution:

MEMPALACE_PYTHON env var (user override)
$PLUGIN_ROOT/venv/bin/python3 (covers dev installs AND Claude Code's per-plugin venv)
System python3 (pip --user / pipx / uv as system install — preserves the original intent of the regressing commits)

.mcp.json goes back to ${CLAUDE_PLUGIN_ROOT}/venv/bin/python -m mempalace.mcp_server. ${CLAUDE_PLUGIN_ROOT} is expanded by Claude Code before spawn, so this works for dev installs (via venv symlink) and packaged plugin installs alike.

Test plan

Editable dev install (pip install -e . in repo venv): both hooks return {} from a neutral cwd; MCP initialize handshake returns serverInfo = {name: mempalace, version: 3.3.1}; tools/list returns all 29 tools.
Fallback to system python3: confirmed by inspection — if neither MEMPALACE_PYTHON nor $PLUGIN_ROOT/venv/bin/python3 resolves, the hook executes the same python3 -m mempalace hook run ... invocation the regressing commits used.
${CLAUDE_PLUGIN_ROOT}/venv/ packaged install path — works by construction (Claude Code's plugin manager creates the venv at that exact location), but not re-tested in this PR.

🤖 Generated with Claude Code

The stop/precompact hooks and .mcp.json switched to PATH-only lookups (`command -v mempalace` / bare `mempalace-mcp`) in recent updates. That works when mempalace is installed system-wide (pipx/uv register the CLI on PATH and put the module on the system python) but breaks two install paths that depend on a local venv: 1. Editable dev installs (`pip install -e .` inside ./venv/) — `mempalace` / `mempalace-mcp` and the module only live in the repo venv, not on PATH. Hooks fall through all three checks and print: "MemPalace hook error: could not find a runnable mempalace command or module" MCP fails to spawn because `mempalace-mcp` is not on PATH. 2. Claude Code plugin installs that create a per-plugin venv at `${CLAUDE_PLUGIN_ROOT}/venv/` — neither the hooks nor .mcp.json look there, so users whose system python doesn't have mempalace get the same errors. Restore the three-tier resolution used prior to the regression: 1. MEMPALACE_PYTHON env var (user override) 2. $PLUGIN_ROOT/venv/bin/python3 (dev installs AND Claude Code's per-plugin venv) 3. system python3 (pip --user / pipx / uv as system install) .mcp.json goes back to `${CLAUDE_PLUGIN_ROOT}/venv/bin/python` with `-m mempalace.mcp_server`. `${CLAUDE_PLUGIN_ROOT}` is expanded by Claude Code before spawn, so this works for dev installs (via venv symlink) and packaged plugin installs (per-plugin venv) alike. Verified against an editable dev install: both hooks return `{}` from a neutral cwd, MCP initialize handshake returns `serverInfo={name: mempalace, version: 3.3.1}` and tools/list returns all 29 tools. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Restores venv-aware invocation for MemPalace’s Claude Code hooks and MCP server startup to support installs where mempalace isn’t available system-wide on PATH (e.g., per-plugin venv).

Changes:

Update stop/precompact hook scripts to resolve a Python interpreter via MEMPALACE_PYTHON, plugin venv, then system python3.
Switch MCP server launch in .claude-plugin/.mcp.json to use the plugin venv Python with -m mempalace.mcp_server.

Reviewed changes

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

File	Description
.claude-plugin/hooks/mempal-stop-hook.sh	Adds interpreter resolution logic and runs hook via `python -m mempalace ...`.
.claude-plugin/hooks/mempal-precompact-hook.sh	Same interpreter resolution change for precompact hook.
.claude-plugin/.mcp.json	Launch MCP server via `${CLAUDE_PLUGIN_ROOT}/venv/bin/python -m mempalace.mcp_server`.

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

Copilot · 2026-04-22T20:16:11Z

+INPUT=$(cat)
+echo "$INPUT" | "$PYTHON" -m mempalace hook run --hook precompact --harness claude-code


The hook currently reads all stdin into INPUT=$(cat) and then pipes it back via echo. This can change the payload (trailing newlines are stripped by command substitution; echo is not safe for arbitrary content) and unnecessarily buffers the entire request in memory. Prefer streaming stdin directly into the Python invocation so JSON is preserved exactly.

Suggested change

INPUT=$(cat)

echo "$INPUT" | "$PYTHON" -m mempalace hook run --hook precompact --harness claude-code

"$PYTHON" -m mempalace hook run --hook precompact --harness claude-code

Copilot · 2026-04-22T20:16:11Z

+if [ -n "$MEMPALACE_PYTHON" ] && [ -x "$MEMPALACE_PYTHON" ]; then
+    PYTHON="$MEMPALACE_PYTHON"
+elif [ -x "$PLUGIN_ROOT/venv/bin/python3" ]; then
+    PYTHON="$PLUGIN_ROOT/venv/bin/python3"
+else
+    PYTHON="python3"
+fi
+INPUT=$(cat)
+echo "$INPUT" | "$PYTHON" -m mempalace hook run --hook stop --harness claude-code


The previous implementation emitted a clear, consistent error when no runnable mempalace command/module was available. With the current logic, if python3 is missing (or if the selected interpreter can't import mempalace), the script will fail with a generic shell/Python error and may not be obvious to users how to fix it. Consider restoring explicit checks (e.g., command -v and a quick -c "import mempalace") and printing the hook-specific error message to stderr before exiting non-zero.

Copilot · 2026-04-22T20:16:12Z

+    PYTHON="$PLUGIN_ROOT/venv/bin/python3"
+else
+    PYTHON="python3"
+fi


The script no longer verifies that the resolved interpreter exists on PATH and can import mempalace before invoking it. If python3 isn't installed (or the chosen interpreter lacks the package), the hook will error with a generic shell/Python message rather than the prior explicit "could not find a runnable mempalace command or module" guidance. Consider adding back explicit checks and a clear stderr error before exiting.

Suggested change

fi

fi

if [[ "$PYTHON" == */* ]]; then

if [ ! -x "$PYTHON" ]; then

echo "mempal-precompact-hook: could not find a runnable mempalace command or module" >&2

exit 1

fi

elif ! command -v "$PYTHON" >/dev/null 2>&1; then

echo "mempal-precompact-hook: could not find a runnable mempalace command or module" >&2

exit 1

fi

if ! "$PYTHON" -c 'import mempalace' >/dev/null 2>&1; then

echo "mempal-precompact-hook: could not find a runnable mempalace command or module" >&2

exit 1

fi

Copilot · 2026-04-22T20:16:12Z

+INPUT=$(cat)
+echo "$INPUT" | "$PYTHON" -m mempalace hook run --hook stop --harness claude-code


The hook currently slurps all stdin into a shell variable and re-emits it via echo. Command substitution strips trailing newlines and echo can subtly transform input, which can corrupt the JSON payload that mempalace hook run expects. Prefer passing stdin through directly to the Python process (no intermediate buffering) so the payload is preserved and large inputs don't get loaded into memory.

Suggested change

INPUT=$(cat)

echo "$INPUT" | "$PYTHON" -m mempalace hook run --hook stop --harness claude-code

"$PYTHON" -m mempalace hook run --hook stop --harness claude-code

jphein · 2026-04-23T22:32:42Z

Closing this as premature on my end — I had explicitly deferred filing in my #1049 comment on 2026-04-21 pending @bensig's arbitration between this autodetect pattern and @sha2fiddy's broader consolidation in #1069. I jumped the gun.

The failing tests in tests/test_claude_plugin_hook_wrappers.py are correctly enforcing the contract #942 established — PATH-only resolution, explicit "could not find a runnable mempalace command or module" error, python fallback after python3. My change reintroduced dirname/cat and dropped those guarantees, which the tests caught in the first run. Working as designed.

Separating the two motivations cleanly:

The .mcp.json piece (bare mempalace-mcp breaks fresh v3.3.2 installs) gets fixed by the v3.3.3 release @bensig mentioned on fix: HNSW graph corruption, PreCompact deadlock, mine fan-out (closes #974, #965, #955) #976 earlier today, via v3.3.2 release defect: mempalace-mcp entry point missing from pyproject.toml despite plugin.json requiring it — MCP server fails to start on fresh install #1093 → pyproject.toml entry point. No longer needs this PR.
The hook-script piece (venv detection for editable dev installs) properly belongs to whatever shape Consolidate hooks/mempal_*_hook.sh into thin wrappers delegating to mempalace hook run #1069 settles on. Re-opening that ahead of arbitration would just repeat this mistake.

Thanks Copilot for the clear inline review — the test-as-spec read was exactly right.

Opened MemPalace#1115 on 2026-04-22 attempting to land the `.claude-plugin/` venv-aware Python resolution upstream; closed it on 2026-04-23 after CI correctly caught the violation of the PATH-only contract MemPalace#942 established, and the realization that the `.claude-plugin/` side of the autodetect work was explicitly waiting on MemPalace#1069 arbitration. - CLAUDE.md PR tracker header: 7 → 8 closed - CLAUDE.md PR tracker gains a MemPalace#1115 closed row with reason - CLAUDE.md fork-ahead row MemPalace#19 language updated (was "Candidate for PR upstream" → now notes the attempt, withdrawal, and gating on bensig's MemPalace#1069 direction) - README.md change queue: Hooks row reflects the MemPalace#1115 attempt + withdrawal; still waiting on arbitration Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…safe graph cache 31 commits including: - v3.3.3 release (restores pip install integrity post-v3.3.2 MemPalace#1093 regression) - MemPalace#1097 empty-string filter normalization in mempalace_search - MemPalace#659 diary wing parameter (our fork PR, now upstream) - MemPalace#851/MemPalace#1097/MemPalace#1021/MemPalace#661/MemPalace#673 incorporated - website Crystal Lattice brand refresh - thread-safe graph cache in palace_graph.py Conflict resolutions (10 files): - README.md keep fork version; bump badge 3.3.1 → 3.3.3 for test compat - hooks/README.md keep fork silent/block architecture docs; keep MEMPAL_PYTHON (correct for legacy hook, upstream's rename is stale) - examples/HOOKS_TUTORIAL.md same treatment - mcp_server.py take upstream's sanitize_name(wing) — strictly better than our crude lowercase+underscore normalization - miner.py keep fork 10K batch size + comma formatting on status; adopt upstream's pagination rationale comment - palace_graph.py take upstream entirely — thread-safety improvements layered on top of our MemPalace#661 (which upstream already merged) - hooks_cli.py take upstream (Windows path-separator compat in _wing_from_transcript_path, Codex CLI format in _extract_recent_messages), then re-apply fork-ahead: use _wing_from_transcript_path in _ingest_transcript instead of hardcoded "sessions" — keeps transcript mining coherent with the diary wing derivation from MemPalace#659 - tests/test_hooks_cli.py take upstream's updated wing-kwarg assertions and new test_stop_hook_derives_wing_from_transcript_path; take upstream's mock-based security test (simpler than our three-way assertion, same property tested) Post-merge test state: - 1096 passed, 10 failed in tests/test_claude_plugin_hook_wrappers.py - The 10 failures are the fork-ahead MemPalace#19 divergence already documented in CLAUDE.md: our venv-aware hooks use `dirname`/`cat` which the test's scrubbed-PATH environment doesn't provide. Same class that correctly caught MemPalace#1115 and led us to withdraw it pending MemPalace#1069 arbitration. Expected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jphein requested a review from bensig as a code owner April 22, 2026 20:12

Copilot AI review requested due to automatic review settings April 22, 2026 20:12

Copilot started reviewing on behalf of jphein April 22, 2026 20:13 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

jphein closed this Apr 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(plugin): restore venv-aware Python resolution for hooks + MCP#1115

fix(plugin): restore venv-aware Python resolution for hooks + MCP#1115
jphein wants to merge 1 commit intoMemPalace:developfrom
jphein:fix/claude-plugin-venv-aware-python

jphein commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

jphein commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		INPUT=$(cat)
		echo "$INPUT" \| "$PYTHON" -m mempalace hook run --hook precompact --harness claude-code

	INPUT=$(cat)
	echo "$INPUT" \| "$PYTHON" -m mempalace hook run --hook precompact --harness claude-code
	"$PYTHON" -m mempalace hook run --hook precompact --harness claude-code

-fi
+fi
+if [[ "$PYTHON" == */* ]]; then
+    if [ ! -x "$PYTHON" ]; then
+        echo "mempal-precompact-hook: could not find a runnable mempalace command or module" >&2
+        exit 1
+    fi
+elif ! command -v "$PYTHON" >/dev/null 2>&1; then
+    echo "mempal-precompact-hook: could not find a runnable mempalace command or module" >&2
+    exit 1
+fi
+if ! "$PYTHON" -c 'import mempalace' >/dev/null 2>&1; then
+    echo "mempal-precompact-hook: could not find a runnable mempalace command or module" >&2
+    exit 1
+fi

	INPUT=$(cat)
	echo "$INPUT" \| "$PYTHON" -m mempalace hook run --hook stop --harness claude-code
	"$PYTHON" -m mempalace hook run --hook stop --harness claude-code

Conversation

jphein commented Apr 22, 2026

Summary

Fix

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

jphein commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants