fix(hooks): resolve Claude plugin hook runner cross-platform by fatkobra · Pull Request #942 · MemPalace/mempalace

fatkobra · 2026-04-16T08:58:55Z

What does this PR do?

Closes #753

Summary

update .claude-plugin/hooks/mempal-stop-hook.sh
update .claude-plugin/hooks/mempal-precompact-hook.sh
prefer the installed mempalace CLI
fall back to importable python3 / python
add execution tests for the wrapper scripts

Scope
This PR intentionally fixes the Claude plugin shell hook wrappers only.
It does not change .claude-plugin/plugin.json or .claude-plugin/.mcp.json,
which still need a separate supported mechanism for MCP command resolution.

How to test

Manual verification

before: both wrappers failed with python3: command not found
after: both wrappers returned {} with exit code 0 in a PATH containing python but no python3

Validation

bash -n .claude-plugin/hooks/mempal-stop-hook.sh
bash -n .claude-plugin/hooks/mempal-precompact-hook.sh
ruff format tests/test_claude_plugin_hook_wrappers.py
ruff check tests/test_claude_plugin_hook_wrappers.py
ruff format --check tests/test_claude_plugin_hook_wrappers.py
python -m pytest tests/test_claude_plugin_hook_wrappers.py -v
python -m pytest tests/test_hooks_cli.py tests/test_claude_plugin_hook_wrappers.py -v
python -m pytest tests/ -v

Checklist

Tests pass (python -m pytest tests/ -v)
No hardcoded paths
Linter passes (ruff check .)

mvalentsev · 2026-04-16T09:44:13Z

~~#833 by igorls already covers this (same issue #753) -- real python resolution with MEMPAL_PYTHON override, all CI green.~~

~~Also #670, #325, #384, #410 target the same problem.~~

fatkobra · 2026-04-16T10:05:33Z

Thanks for the pointer @mvalentsev .

I checked #833, and it looks complementary rather than identical:

fix(hooks): real python-resolution for .sh hooks, with MEMPAL_PYTHON override #833 updates the legacy shell hooks in hooks/mempal_save_hook.sh and hooks/mempal_precompact_hook.sh
this PR updates the Claude plugin wrapper scripts in .claude-plugin/hooks/mempal-stop-hook.sh and .claude-plugin/hooks/mempal-precompact-hook.sh

Issue #753’s current code example is the wrapper path (python3 -m mempalace hook run --hook stop --harness claude-code), which is what this PR fixes.

If maintainers would prefer a single combined solution, I’m happy to close this and rework it on top of #833, but as written it looks like #942 fixes the plugin wrapper layer while #833 fixes the legacy save/precompact shell hooks.

mvalentsev · 2026-04-16T10:12:01Z

You're right, my mistake -- #833 targets hooks/ (legacy scripts) while this PR targets .claude-plugin/hooks/ (plugin wrappers). Different files, complementary fixes. Thanks for checking.

mvalentsev · 2026-04-16T10:20:28Z

btw, .codex-plugin/hooks/mempal-hook.sh has the same hardcoded python3 on line 6. Might be worth including in this PR since it's the same class of fix.

mvalentsev · 2026-04-16T10:27:18Z

Also worth noting: the plugin JSON configs (.claude-plugin/plugin.json, .mcp.json, .codex-plugin/plugin.json) also hardcode python3 as the MCP server command. Those need an entry point fix rather than shell fallbacks -- #805 and #340 both address that side of things.

fatkobra · 2026-04-16T11:15:02Z

Thanks — agreed on both points.

.codex-plugin/hooks/mempal-hook.sh does look like the same wrapper-layer issue, so I’m happy to add that here if maintainers want this PR to cover all plugin hook wrappers.

However, I think it is better to keep the MCP config files (.claude-plugin/plugin.json, .claude-plugin/.mcp.json, .codex-plugin/plugin.json) out of scope for this PR, since those seem to need the separate entry-point/config approach already being discussed in #805 / #340.

fatkobra · 2026-04-18T16:42:08Z

I have a tested Codex-wrapper follow-up patch ready locally as well (.codex-plugin/hooks/mempal-hook.sh) if maintainers @igorls would prefer this PR to cover all plugin hook wrappers.

Qodo-Free-For-OSS · 2026-04-19T06:26:10Z

Hi, The wrapper execution tests pass a Windows-formatted path (backslashes/drive letters) directly to bash, which commonly fails under Git Bash/MSYS path semantics. This can break the Windows CI job whenever bash exists on PATH (tests are only skipped when bash is missing).

Severity: action required | Category: reliability

How to fix: Use POSIX path for bash

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

tests/test_claude_plugin_hook_wrappers.py invokes bash with a script path built via str(Path(...)). On Windows this produces backslashes, which Git Bash/MSYS often cannot resolve.

Issue Context

The tests are skipped only when bash is missing. Windows CI (windows-latest) frequently has Git Bash available, so these tests will run and can fail.

Fix Focus Areas

tests/test_claude_plugin_hook_wrappers.py[25-69]

Convert the script path passed to bash using the existing _shell_path() helper (or otherwise normalize to POSIX/MSYS-acceptable form).

We noticed a couple of other issues in this PR as well - happy to share if helpful.

Found by Qodo code review

fatkobra · 2026-04-19T08:45:30Z

Thanks — fixed. _run_hook() now passes the wrapper script path to bash through _shell_path(...) instead of str(Path(...)), so the tests use POSIX-style paths on Windows/Git Bash too.

Happy to look at the other findings too if you can share the specific issues or exact lines involved. I’d prefer to keep follow-up changes concrete and in-scope for this PR. @qodo-ai-reviewer

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>

@messelink

Restore-integrity release. Unbreaks fresh `pip install mempalace` from v3.3.2 by re-tagging current develop, which carries both the plugin.json consumer (shipped in 3.3.2) and the matching mempalace-mcp entry point in pyproject.toml (added on develop ~10h after the 3.3.2 tag via MemPalace#340 by @messelink). MemPalace#1093 diagnosed by @jphein. Bumps (all 5 sources agree per Version Guard / CLAUDE.md): - mempalace/version.py 3.3.2 → 3.3.3 - pyproject.toml 3.3.2 → 3.3.3 - .claude-plugin/plugin.json 3.3.2 → 3.3.3 - .claude-plugin/marketplace.json 3.3.2 → 3.3.3 - .codex-plugin/plugin.json 3.3.2 → 3.3.3 - CHANGELOG.md new [3.3.3] entry No code changes. The fix for MemPalace#1093 is already on develop via merged PRs MemPalace#340, MemPalace#1021, MemPalace#851, MemPalace#942, MemPalace#833, MemPalace#673, MemPalace#661, MemPalace#659, MemPalace#1097, MemPalace#1051, MemPalace#1001, MemPalace#945. Branch name intentionally outside the `release/*` ruleset so follow-up CI-fix commits aren't gated behind a nested PR. (Supersedes MemPalace#1143 — closed for exactly that reason after it missed 3 of 5 version files.) Smoke-tested locally from a fresh develop clone: grep mempalace-mcp pyproject.toml .claude-plugin/plugin.json # both ✓ python -m build --wheel # ✓ pip install …-py3-none-any.whl # ✓ which mempalace-mcp # ✓ mempalace-mcp --help # ✓

@igorls

… state Three stale sections updated: - Fork change queue: row 8 (.blob_seq_ids_migrated marker) struck through → FILED as MemPalace#1177. Two new rows added for segfault fixes discovered today (MemPalace#1171 concurrent-write lock, MemPalace#1173 quarantine in make_client) that weren't in the queue because the bugs surfaced today, not during the original 2026-04-21 triage. - Open upstream PRs: was showing 3 of 10 PRs. Now shows all 10 with current CI/review state. All rebased onto current upstream/develop and MERGEABLE as of today. - Merged since v3.3.1: added v3.3.3 release (2026-04-24) with its constituent merges — MemPalace#942, MemPalace#833, MemPalace#1097, MemPalace#1145, MemPalace#1147, MemPalace#1148/1150/1157 entity-detection overhaul (via @igorls's MemPalace#1175 stacked-PR rescue), MemPalace#1166 palace-path security, MemPalace#340/MemPalace#1093 install regression, plus MemPalace#851 from the 2026-04-22 batch.

fatkobra added 3 commits April 16, 2026 10:32

Create test_claude_plugin_hook_wrappers.py

e083cd6

Update mempal-stop-hook.sh

5fe0c1c

Update mempal-precompact-hook.sh

be9214a

fatkobra requested review from bensig, igorls and milla-jovovich as code owners April 16, 2026 08:58

test: make Claude plugin wrapper tests portable on Windows

1dc55a7

mvalentsev mentioned this pull request Apr 18, 2026

mempal-stop-hook.sh fails on Windows: python3 command not found #997

Closed

test: normalize wrapper script path for bash on Windows

0b316d4

mvalentsev mentioned this pull request Apr 20, 2026

Hooks and MCP server break in projects with Python venv (hardcoded python3) #1049

Open

igorls merged commit 4fb0ee5 into MemPalace:develop Apr 21, 2026
6 checks passed

sha2fiddy mentioned this pull request Apr 21, 2026

Consolidate hooks/mempal_*_hook.sh into thin wrappers delegating to mempalace hook run #1069

Open

jphein mentioned this pull request Apr 23, 2026

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

Closed

3 tasks

This was referenced Apr 23, 2026

release: v3.3.3 — restore install integrity #1143

Closed

release: v3.3.3 — restore install integrity #1144

Merged

bensig mentioned this pull request Apr 24, 2026

release: v3.3.3 — sync develop → main for tag cut #1159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(hooks): resolve Claude plugin hook runner cross-platform#942

fix(hooks): resolve Claude plugin hook runner cross-platform#942
igorls merged 5 commits intoMemPalace:developfrom
fatkobra:fix-hooks-resolve-claude-plugin

fatkobra commented Apr 16, 2026

Uh oh!

mvalentsev commented Apr 16, 2026 •

edited

Loading

Uh oh!

fatkobra commented Apr 16, 2026

Uh oh!

mvalentsev commented Apr 16, 2026

Uh oh!

mvalentsev commented Apr 16, 2026

Uh oh!

mvalentsev commented Apr 16, 2026

Uh oh!

fatkobra commented Apr 16, 2026

Uh oh!

fatkobra commented Apr 18, 2026

Uh oh!

Qodo-Free-For-OSS commented Apr 19, 2026

Issue description

Issue Context

Fix Focus Areas

Uh oh!

fatkobra commented Apr 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

fatkobra commented Apr 16, 2026

What does this PR do?

How to test

Checklist

Uh oh!

mvalentsev commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fatkobra commented Apr 16, 2026

Uh oh!

mvalentsev commented Apr 16, 2026

Uh oh!

mvalentsev commented Apr 16, 2026

Uh oh!

mvalentsev commented Apr 16, 2026

Uh oh!

fatkobra commented Apr 16, 2026

Uh oh!

fatkobra commented Apr 18, 2026

Uh oh!

Qodo-Free-For-OSS commented Apr 19, 2026

Issue description

Issue Context

Fix Focus Areas

Uh oh!

fatkobra commented Apr 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mvalentsev commented Apr 16, 2026 •

edited

Loading