From 045023f4494afb1a5c0ba71ff112293b17384b96 Mon Sep 17 00:00:00 2001 From: Milla J <232237854+milla-jovovich@users.noreply.github.com> Date: Mon, 13 Apr 2026 18:09:59 -0700 Subject: [PATCH] fix: save hook auto-mines transcript without MEMPAL_DIR (#840) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TDD: test written first, failed, then fixed. Problem: save hook says "saved in background" but MEMPAL_DIR defaults to empty, so nothing actually mines. Users get no auto-save despite the hook firing every 15 messages. Fix: use TRANSCRIPT_PATH (received from Claude Code in the hook's JSON input) to discover the session directory. Mine that directory automatically. MEMPAL_DIR is still supported as override but no longer required. Also fixed: bare python3 → $(command -v python3) for nohup safety. Co-authored-by: Claude Opus 4.6 (1M context) --- hooks/mempal_save_hook.sh | 17 ++++++--- tests/test_save_hook_mines.py | 68 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 tests/test_save_hook_mines.py diff --git a/hooks/mempal_save_hook.sh b/hooks/mempal_save_hook.sh index b15d961..9eda976 100755 --- a/hooks/mempal_save_hook.sh +++ b/hooks/mempal_save_hook.sh @@ -133,11 +133,20 @@ if [ "$SINCE_LAST" -ge "$SAVE_INTERVAL" ] && [ "$EXCHANGE_COUNT" -gt 0 ]; then echo "[$(date '+%H:%M:%S')] TRIGGERING SAVE at exchange $EXCHANGE_COUNT" >> "$STATE_DIR/hook.log" - # Optional: run mempalace ingest in background if MEMPAL_DIR is set + # Auto-mine the transcript. Two paths: + # 1. TRANSCRIPT_PATH (from Claude Code) — mine the directory it lives in + # 2. MEMPAL_DIR (user-configured) — mine that directory + # At least one should work. If neither is set, nothing mines. + PYTHON="$(command -v python3)" + MINE_DIR="" + if [ -n "$TRANSCRIPT_PATH" ] && [ -f "$TRANSCRIPT_PATH" ]; then + MINE_DIR="$(dirname "$TRANSCRIPT_PATH")" + fi if [ -n "$MEMPAL_DIR" ] && [ -d "$MEMPAL_DIR" ]; then - SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - REPO_DIR="$(dirname "$SCRIPT_DIR")" - python3 -m mempalace mine "$MEMPAL_DIR" >> "$STATE_DIR/hook.log" 2>&1 & + MINE_DIR="$MEMPAL_DIR" + fi + if [ -n "$MINE_DIR" ]; then + "$PYTHON" -m mempalace mine "$MINE_DIR" >> "$STATE_DIR/hook.log" 2>&1 & fi # Notify the AI that a checkpoint happened — but do NOT ask it to write diff --git a/tests/test_save_hook_mines.py b/tests/test_save_hook_mines.py new file mode 100644 index 0000000..a702a42 --- /dev/null +++ b/tests/test_save_hook_mines.py @@ -0,0 +1,68 @@ +"""TDD: save hook must actually mine conversations without MEMPAL_DIR. + +The save hook should auto-discover the conversation transcript and mine it +without the user needing to set MEMPAL_DIR. Currently MEMPAL_DIR defaults +to empty, which means the mining block is skipped and nothing is saved +despite the hook telling the agent "saved in background." + +Written BEFORE the fix. +""" + +import os + + +class TestSaveHookAutoMines: + """The save hook must mine the active transcript automatically.""" + + def test_hook_mines_transcript_path(self): + """The hook receives TRANSCRIPT_PATH from Claude Code. + It should use that to mine the conversation, not depend on MEMPAL_DIR.""" + hook_path = os.path.join( + os.path.dirname(os.path.dirname(__file__)), + "hooks", + "mempal_save_hook.sh", + ) + src = open(hook_path).read() + + # The hook ALREADY receives TRANSCRIPT_PATH in the JSON input. + # It should use this to mine the current session's transcript + # regardless of whether MEMPAL_DIR is set. + # The hook must have a path that uses TRANSCRIPT_PATH to determine + # what to mine, separate from the MEMPAL_DIR path. + uses_transcript = "TRANSCRIPT_PATH" in src + has_mine = "mempalace mine" in src + # TRANSCRIPT_PATH must appear in the mining logic, not just the parse block + transcript_drives_mine = "MINE_DIR" in src and "dirname" in src and "TRANSCRIPT_PATH" in src + + assert uses_transcript and has_mine and transcript_drives_mine, ( + "Save hook only mines when MEMPAL_DIR is set (defaults to empty). " + "The hook receives TRANSCRIPT_PATH from Claude Code — it should " + "mine that file automatically so conversations are saved without " + "the user setting an env var. Currently the hook says 'saved in " + "background' but nothing actually saves." + ) + + def test_mempal_dir_default_not_empty(self): + """If MEMPAL_DIR is still used, it should have a sensible default, + not an empty string that silently disables mining.""" + hook_path = os.path.join( + os.path.dirname(os.path.dirname(__file__)), + "hooks", + "mempal_save_hook.sh", + ) + src = open(hook_path).read() + + # Check if MEMPAL_DIR defaults to empty + has_empty_default = 'MEMPAL_DIR=""' in src + + # If it defaults to empty, mining is silently disabled + if has_empty_default: + # There must be an alternative mining path that doesn't need MEMPAL_DIR + has_alternative = ( + src.count("mempalace mine") > 1 + or "TRANSCRIPT_PATH" in src.split("mempalace mine")[0] + ) + assert has_alternative, ( + 'MEMPAL_DIR defaults to "" which silently disables mining. ' + "Either set a default path or add transcript-based mining." + )