Bundle hook scripts in pip package

[armujahid]

Summary

• Breaking change: hooks/ moved to mempalace/hooks/ — users referencing the old repo-root path must update their config
• Hook scripts now ship with pip install mempalace
• Add mempalace hooks path and mempalace hooks install CLI subcommands for easy setup
• mempalace hooks install prints ready-to-paste config for Claude Code or Codex CLI (--format codex)

Closes #184

Breaking change

The hooks/ directory at the repo root has been moved into mempalace/hooks/. Anyone pointing their Claude Code or Codex config at the old hooks/ path needs to either:

1. Update the path manually, or
2. Run mempalace hooks install to get a config snippet with the correct paths

Test plan

• uv run mempalace hooks path prints the hooks directory
• uv run mempalace hooks install outputs valid JSON config for Claude Code
• uv run mempalace hooks install --format codex outputs valid JSON config for Codex CLI
• uv run pytest tests/test_hooks.py -v — all 8 tests pass
• uv build && unzip -l dist/*.whl | grep hooks — both .sh files present in wheel

[bgauryy]

PR #265 — Bundle hook scripts in pip package

Field	Value
Title	Bundle hook scripts in pip package
Author	armujahid (Abdul Rauf)
Branch	fix/hooks-in-pip-package → main
Commits	1 (6070a96b)
Files	6 changed (+179, -0 net — renames count as 0 deletions)
Closes	#184

---

Summary

Moves the hooks/ directory from the repo root into mempalace/hooks/ so it ships with pip install mempalace. Adds a mempalace hooks CLI subcommand with two actions:

• path — prints the installed hooks directory (or a specific hook file path)
• install — prints ready-to-paste JSON config for Claude Code or Codex CLI

Verdict: Approve with suggestions

Clean, well-scoped, single-commit PR. The feature is genuinely useful — users currently need to clone the repo or manually copy hook scripts. Shipping them inside the package and providing mempalace hooks install is the right UX. Tests are solid.

There are two issues to address before merging and a handful of minor suggestions.

---

Issues

1. MEDIUM: .sh files may not be included in wheel without explicit package-data

The pyproject.toml has no [tool.setuptools.package-data] section. Modern setuptools defaults include-package-data = true and includes VCS-tracked files, so this works when building from a git checkout. However:

• Source tarballs from PyPI (sdist) don't carry git metadata — .sh files could be silently dropped
• The implicit behavior is fragile and non-obvious to future maintainers

The author verified the wheel includes the scripts (uv build && unzip -l dist/*.whl | grep hooks), but an explicit declaration is safer.

Suggested fix — add to pyproject.toml:

[tool.setuptools.package-data]
"mempalace.hooks" = ["*.sh", "README.md"]

2. LOW-MEDIUM: importlib.resources.files() returns a Traversable, not a Path

def hooks_dir() -> Path:
    return Path(str(importlib.resources.files("mempalace.hooks")))

importlib.resources.files() returns a Traversable — Path(str(...)) works for on-disk installs but breaks for zip-imported packages (.pex, .egg). Since mempalace is a CLI tool always installed via pip to disk, this is acceptable in practice, but worth a docstring note.

---

Suggestions

3. Test assertion is too loose

# test_hooks.py:48
assert result.stdout.strip().endswith("hooks")

This matches any path ending in "hooks" (e.g. /usr/local/lib/python3.12/webhooks). More specific:

assert result.stdout.strip().endswith("mempalace/hooks")

4. Mixed human text + JSON in install output

The install command prints a header line ("Add to .claude/settings.local.json:\n") then JSON. Users wanting to pipe output to a file must strip the header. Consider a --raw flag that prints only JSON, or use stderr for the header and stdout for the JSON.

The tests already have to work around this (lines[2:] to skip header + blank line), which is a code smell.

5. Claude Code config: PreCompact missing matcher field

"PreCompact": [
    {
        "hooks": [
            {"type": "command", "command": precompact, "timeout": 30}
        ],
    }
],

The Stop section includes "matcher": "*" but PreCompact does not. This matches the original hooks/README.md convention, but it's inconsistent. If matcher is optional and defaults to "*", document that; otherwise add it.

6. No runtime migration warning

The README documents the breaking change (old hooks/ path stops working), but users who upgrade via pip won't see the README. Consider printing a one-time warning in mempalace CLI startup if the old hooks/ directory exists at the repo root, pointing them to mempalace hooks install.

---

What's good

• Single responsibility: one commit, one feature, clean diff
• Correct use of importlib.resources for locating installed package data (compatible with Python 3.9+ as required)
• Both config formats (Claude Code + Codex CLI) covered
• 8 tests covering the module API, CLI integration, and executable bit — good coverage
• Breaking change clearly documented in PR body with migration steps
• README updated in the same commit — self-contained change

---

File-by-file

File	Lines	Notes
mempalace/hooks/__init__.py	+20	Clean. hooks_dir() and hook_path() are the right API
mempalace/cli.py	+72	cmd_hooks + argparse wiring. Well-structured
mempalace/hooks/README.md	+12	Quick-setup section added at top. Good UX
hooks/*.sh → mempalace/hooks/*.sh	0	Pure renames, no content change. Correct
tests/test_hooks.py	+75	8 tests. Uses subprocess for CLI tests. Solid
pyproject.toml	0	Missing — needs package-data for .sh files (see issue #1)

---

Created by Octocode MCP https://octocode.ai

[armujahid]

@bgauryy Thanks for the thorough review! Pushed a follow-up commit addressing items 3 and 4.

Addressed:

• Item 3 (Test assertion too loose): Tightened to endswith("mempalace/hooks")
• Item 4 (Mixed text + JSON): Header now prints to stderr, stdout is pure JSON — pipeable via mempalace hooks install > config.json. Tests simplified accordingly (no more lines[2:] workaround).

Not addressing:

• Item 1 (Missing package-data): This project uses hatchling, not setuptools. Hatchling auto-includes all files under listed package directories — the [tool.setuptools.package-data] config would be silently ignored. We verified the wheel contains both .sh files via uv build && unzip -l dist/*.whl | grep hooks.
• Item 2 (Traversable vs Path): As you noted, this is fine in practice — mempalace is a CLI tool, always installed to disk via pip.
• Item 5 (PreCompact missing matcher): This matches the original hooks/README.md convention. The matcher field is specific to Stop hooks for tool pattern matching — PreCompact doesn't use it.
• Item 6 (Runtime migration warning): Users with the old hooks/ path would have cloned the repo and can see the change via git. Pip-only users never had hooks before, so there's nothing to migrate.

Would appreciate another look when you get a chance!

[armujahid]

Rebased. uv.lock looks also stale that I didn't update in this PR

[web3guru888]

👀 Review of #265 — Bundle hook scripts in pip package

Scope: +175/−0 · 6 file(s)

• mempalace/cli.py (modified: +72/−0)
• mempalace/hooks/README.md (renamed: +12/−0)
• mempalace/hooks/__init__.py (added: +20/−0)
• mempalace/hooks/mempal_precompact_hook.sh (renamed: +0/−0)
• mempalace/hooks/mempal_save_hook.sh (renamed: +0/−0)
• tests/test_hooks.py (added: +71/−0)

Technical Analysis

• 🪟 Windows compatibility — verify path handling works cross-platform

Strengths

• ✅ Includes test coverage

🟢 Approved — clean, well-structured PR. Good work @armujahid!

---

_{🏛️ Reviewed by MemPalace-AGI · Autonomous research system with perfect memory · Showcase: Truth Palace of Atlantis}

[igorls]

Hi, thanks for the contribution.

This PR has merge conflicts with develop, and the branch has not been updated in over 7 days, which puts it before our most recent release. The conflicts are likely against work that landed in that release.

Could you rebase onto develop so we can take another look?

If this change is no longer relevant, feel free to close the PR.

(This message is part of a periodic backlog pass, sent to all open PRs that match this state.)

[armujahid]

@igorls done.

[igorls]

Thanks for this contribution, and apologies for the slow turnaround.

develop has moved a fair way since this was opened and the branch no longer merges cleanly. If you're still interested in landing it, could you rebase onto current develop? Once it merges cleanly and CI is green I'll get it reviewed for the 3.8.0 cycle.

If you'd rather not pick it back up, no problem at all — just say so and I'll close it out, and thanks either way for taking the time to send it.

[armujahid]

@igorls Done. I have resolved merge conflicts. You can trigger CI workflows.

[igorls]

Thanks for rebasing so quickly. CI ran — linux 3.9/3.11/3.13, macOS, smoke, build and build-gpu are all green. Three things to fix, all small:

1. ruff format

unformatted: File would be reformatted
  --> tests/test_save_hook_mines.py:81:29

uv run ruff format tests/test_save_hook_mines.py clears it. ruff check itself passed.

2. test_hook_scripts_are_executable fails on Windows

mode & stat.S_IXUSR has no meaning on NTFS, so this can only ever fail there. Worth a skip:

@pytest.mark.skipif(sys.platform == "win32", reason="POSIX permission bits")

3. test_cli_hooks_path fails on Windows

result.stdout.strip().endswith("mempalace/hooks") assumes a forward slash. Comparing paths rather than strings avoids it:

assert Path(result.stdout.strip()) == hooks_dir()

---

Two notes from reviewing the rest, neither of them blocking:

I wanted to be sure the executable bit survived a real wheel build rather than only the source checkout, so I built the branch and read the mode bits out of the zip — all three main hooks come through -rwxr-xr-x, and the two lib/common.sh files that don't are sourced rather than invoked, so that's correct. Nice that hatchling's artifacts preserves it; that was the part I most expected to bite.

On the breaking change — thank you for leading the description with it rather than burying it. One thing I'd like to think about before this lands: because hooks run in the background, a stale path in someone's settings.json doesn't raise anything they'll see. Their memories just quietly stop being filed. That's the failure mode this project can least afford, so at minimum this needs a prominent release note, and I want to consider whether a temporary shim at the old hooks/ location is worth carrying for a release. That's my call to make, not extra work for you — no changes needed from your side unless I decide the shim is worth it.

Push those three and I'll take a proper look at the packaging change.

[armujahid]

@igorls Thanks for the review. All three points are already addressed in my last commit

• Applied Ruff formatting.
• Skipped the POSIX executable-bit assertion on Windows.
• Made the hook path assertion platform-independent.