Skip to content

fix: replace bash wrapper hooks with direct mempalace commands (Windows path fix) - #1661

Open
w1tc4 wants to merge 1 commit into
MemPalace:developfrom
w1tc4:fix/windows-hooks-bash-path-mangling
Open

fix: replace bash wrapper hooks with direct mempalace commands (Windows path fix)#1661
w1tc4 wants to merge 1 commit into
MemPalace:developfrom
w1tc4:fix/windows-hooks-bash-path-mangling

Conversation

@w1tc4

@w1tc4 w1tc4 commented May 30, 2026

Copy link
Copy Markdown

Problem

On Windows, the Stop and PreCompact hooks fail with:

Stop hook error: /bin/bash: C:UsersRachael.claudepluginscachemempalacemempalace3.3.6/hooks/mempal-stop-hook.sh: No such file or directory

CLAUDE_PLUGIN_ROOT on Windows expands to a backslash path (C:\Users\...). When passed to bash, the backslashes are treated as escape sequences and stripped, producing a non-existent path.

Fix

The .sh wrappers are single-line pass-throughs to mempalace hook run. Replace the bash wrapper calls with the direct command, which works cross-platform since mempalace is on PATH after installation.

Before:

"command": "bash \"${CLAUDE_PLUGIN_ROOT}/hooks/mempal-stop-hook.sh\""

After:

"command": "mempalace hook run --hook stop --harness claude-code"

Same for PreCompact.

Testing

Verified on Windows 11 with Claude Code v2.1.157 — Stop and PreCompact hooks run cleanly after this change.

Closes #1660

@w1tc4
w1tc4 requested a review from milla-jovovich as a code owner May 30, 2026 02:47

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the plugin hooks to use the mempalace hook run command, introduces a configurable persistence directory for ChromaDB by reading backend.persist_directory from mempalace.yaml, and updates the configuration saving logic in room_detector_local.py to preserve existing keys (like backend or storage) instead of overwriting them. There are no review comments, so I have no feedback to provide.

@igorls

igorls commented May 30, 2026

Copy link
Copy Markdown
Member

Two confirmed blockers, plus a routine rebase, before this can go in:

  1. PreCompact hook command is broken (one-char fix). .claude-plugin/hooks/hooks.json:20 calls mempalace hook run --hook pre-compact …, but cli.py:1457 only accepts choices=["session-start","stop","precompact"] — no hyphen. Verified: mempalace hook run --hook pre-compact … exits 2 with invalid choice: 'pre-compact'. So this change doesn't fix PreCompact, it makes it fail on every platform. It must be --hook precompact (the Stop line, --hook stop, is already correct).

  2. Out-of-scope, half-wired persist_directory feature. The PR is described as a hooks fix, but it also adds _resolve_persist_dir() and reroutes the writer to backend.persist_directory while many readers still hardcode os.path.join(palace_path, "chroma.sqlite3") (e.g. searcher.py:308, mcp_server.py:742, plus repair/migrate/palace). If anyone sets persist_directory, the DB is written in one place while search/status/repair look in another → split-brain palace + false "no palace". This is the same defect as fix(backend): respect backend.persist_directory from palace mempalace.yaml #1658. Please split it into its own PR, wired end-to-end + tested, or drop it here.

Also needs a rebase onto current develop (real conflict in chroma.py _prepare_palace_for_open). To be clear — I checked and merging this does not revert #1617 (that was a 2-dot-diff artifact from the stale base), so no regression there; it's just a normal rebase.

The hooks-only change, corrected to precompact and kept as a thin wrapper, is a ~1-line fix and very welcome.

w1tc4 pushed a commit to w1tc4/mempalace that referenced this pull request May 30, 2026
…ory branch

Resolve conflict in chroma.py: keep module-level _resolve_persist_dir
with lru_cache (our implementation) rather than the inline class-level
duplicate from MemPalace#1661. Keep _fix_missing_collection_type call that MemPalace#1661
did not include.
@w1tc4
w1tc4 force-pushed the fix/windows-hooks-bash-path-mangling branch from cdab7a2 to 81a0f8f Compare May 30, 2026 20:39
…ws path fix)

On Windows, CLAUDE_PLUGIN_ROOT expands to a backslash path. When passed
to bash, the backslashes are treated as escape sequences and stripped,
producing a non-existent path and breaking Stop/PreCompact hooks.

Replace the bash wrapper calls with direct mempalace commands (which work
cross-platform since mempalace is on PATH after installation).

Also fix PreCompact hook arg: was --hook pre-compact (invalid choice),
must be --hook precompact (no hyphen) to match cli.py choices.

Closes MemPalace#1660
@w1tc4
w1tc4 force-pushed the fix/windows-hooks-bash-path-mangling branch from 81a0f8f to 05899f8 Compare June 11, 2026 21:54
@w1tc4

w1tc4 commented Jun 11, 2026

Copy link
Copy Markdown
Author
  • .claude-plugin/hooks/hooks.json only — 2 lines changed
  • bash "..." → mempalace hook run --hook stop/precompact --harness claude-code
  • pre-compact → precompact fixed
  • No persist_dir code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: plugin hooks.json bash wrapper mangles paths, causing Stop/PreCompact hooks to fail

2 participants