From d18e02c91cb968b65d58bc0255b6e93a3ba82db5 Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 25 Apr 2026 16:46:41 +0200 Subject: [PATCH 1/5] fix: centralize .remember/ directory bootstrap, remove hooks.json 2>> redirect The SessionStart and PostToolUse hook commands in hooks.json redirected stderr to .remember/logs/hook-errors.log via 2>>. Bash opens the redirect target before running the script, so on fresh projects where .remember/logs/ does not exist, the redirect fails and the hook script never executes. Four independent users reported this (issues #23, #27, #31, #32). Fix: introduce scripts/bootstrap-dirs.sh as the single source of truth for the .remember/ directory layout. Every hook script sources it after resolve-paths.sh. It creates tmp/, logs/, logs/autonomous/, .gitignore, and redirects stderr via exec 2>> (guarded by -d check for read-only filesystems). hooks.json commands are now clean one-liners with no inline redirects. - New: scripts/bootstrap-dirs.sh (dir creation + stderr redirect) - session-start-hook.sh: source bootstrap-dirs.sh, remove duplicate mkdir - post-tool-hook.sh: source bootstrap-dirs.sh - hooks.json: remove 2>> redirects from both commands - 17 new tests covering: bug reproduction, fresh project, partial state, spaces/unicode in paths, read-only filesystem, idempotency, git worktrees, concurrent sessions, source ordering Closes #23, #27, #31, #32 Supersedes #21 Co-Authored-By: Max --- hooks/hooks.json | 4 +- scripts/bootstrap-dirs.sh | 43 +++ scripts/post-tool-hook.sh | 1 + scripts/session-start-hook.sh | 5 +- tests/test_path_resolution.py | 532 ++++++++++++++++++++++++++++++++++ 5 files changed, 579 insertions(+), 6 deletions(-) create mode 100644 scripts/bootstrap-dirs.sh diff --git a/hooks/hooks.json b/hooks/hooks.json index 844450d..56d2621 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -5,7 +5,7 @@ "hooks": [ { "type": "command", - "command": "bash \"${CLAUDE_PLUGIN_ROOT}/scripts/session-start-hook.sh\" 2>> \"${CLAUDE_PROJECT_DIR:-.}/.remember/logs/hook-errors.log\"" + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/scripts/session-start-hook.sh\"" } ] } @@ -15,7 +15,7 @@ "hooks": [ { "type": "command", - "command": "bash \"${CLAUDE_PLUGIN_ROOT}/scripts/post-tool-hook.sh\" 2>> \"${CLAUDE_PROJECT_DIR:-.}/.remember/logs/hook-errors.log\"" + "command": "bash \"${CLAUDE_PLUGIN_ROOT}/scripts/post-tool-hook.sh\"" } ] } diff --git a/scripts/bootstrap-dirs.sh b/scripts/bootstrap-dirs.sh new file mode 100644 index 0000000..2332a01 --- /dev/null +++ b/scripts/bootstrap-dirs.sh @@ -0,0 +1,43 @@ +#!/bin/bash +# ============================================================================ +# bootstrap-dirs.sh — Single source of truth for .remember/ directory layout +# ============================================================================ +# +# DESCRIPTION +# Creates the .remember/ directory structure and sets up stderr logging. +# Every hook script sources this after resolve-paths.sh to guarantee the +# directory tree exists before any file I/O. +# +# This replaces the inline 2>> redirect that was in hooks.json, which +# failed on fresh projects because bash opens the redirect target before +# the script runs (chicken-and-egg bug: GitHub issues #23, #27, #31, #32). +# +# USAGE +# source "$(dirname "$0")/resolve-paths.sh" +# source "$(dirname "$0")/bootstrap-dirs.sh" +# +# REQUIRES +# PROJECT_DIR must be set (by resolve-paths.sh) +# +# ============================================================================ + +REMEMBER_DIR="${PROJECT_DIR}/.remember" + +# --- Create directory structure --- +mkdir -p \ + "$REMEMBER_DIR/tmp" \ + "$REMEMBER_DIR/logs" \ + "$REMEMBER_DIR/logs/autonomous" \ + 2>/dev/null + +# --- Gitignore so .remember/ never gets committed --- +[ -f "$REMEMBER_DIR/.gitignore" ] || echo '*' > "$REMEMBER_DIR/.gitignore" 2>/dev/null + +# --- Redirect stderr to hook-errors.log --- +# This replaces the 2>> that was in hooks.json. Now the directory is +# guaranteed to exist before we open the file. +# Guard: only redirect if the logs dir was actually created (read-only +# filesystems, Docker read-only mounts, etc. will skip this gracefully). +if [ -d "$REMEMBER_DIR/logs" ]; then + exec 2>> "$REMEMBER_DIR/logs/hook-errors.log" +fi diff --git a/scripts/post-tool-hook.sh b/scripts/post-tool-hook.sh index 091f481..fbaf625 100755 --- a/scripts/post-tool-hook.sh +++ b/scripts/post-tool-hook.sh @@ -29,6 +29,7 @@ # --- Resolve paths --- source "$(dirname "$0")/resolve-paths.sh" +source "$(dirname "$0")/bootstrap-dirs.sh" source "$(dirname "$0")/detect-tools.sh" PLUGIN_ROOT="$PIPELINE_DIR" PROJECT="$PROJECT_DIR" diff --git a/scripts/session-start-hook.sh b/scripts/session-start-hook.sh index d6e2bc0..0bb9fc4 100755 --- a/scripts/session-start-hook.sh +++ b/scripts/session-start-hook.sh @@ -38,6 +38,7 @@ # ============================================================================ source "$(dirname "$0")/resolve-paths.sh" +source "$(dirname "$0")/bootstrap-dirs.sh" source "$(dirname "$0")/detect-tools.sh" PLUGIN_ROOT="$PIPELINE_DIR" PROJECT="$PROJECT_DIR" @@ -54,10 +55,6 @@ dispatch "before_session_start" # ── Cleanup + health check ───────────────────────────────────────────────── rm -f "$PROJECT/.remember/tmp/save-session.pid" -for DIR in "$PROJECT/.remember/tmp" "$PROJECT/.remember/logs" "$PROJECT/.remember/logs/autonomous"; do - mkdir -p "$DIR" 2>/dev/null -done -[ -f "$PROJECT/.remember/.gitignore" ] || echo '*' > "$PROJECT/.remember/.gitignore" # ── Recovery: save the most recent missed session ────────────────────────── if [ "$(cfg '.features.recovery' true)" = "true" ]; then diff --git a/tests/test_path_resolution.py b/tests/test_path_resolution.py index eb7d535..cdeec90 100644 --- a/tests/test_path_resolution.py +++ b/tests/test_path_resolution.py @@ -1346,3 +1346,535 @@ def test_issue_11_all_points_summary(self): 6. CRLF → test_safe_eval_with_crlf, _crlf_arithmetic """ pass # All assertions are in the individual tests above + + +@pytest.mark.skipif(not _has_resolve_paths(), reason="resolve-paths.sh not yet created") +class TestFreshProjectBootstrap: + """GitHub issues #23, #27, #31, #32: hooks fail on fresh projects. + + When .remember/logs/ doesn't exist, the 2>> redirect in hooks.json + fails before the script runs — a chicken-and-egg bug. Scripts must + bootstrap their own directory structure instead of relying on the + caller to pre-create it. + """ + + def test_current_hooks_json_fails_without_remember_dir(self, tmp_path): + """BUG REPRODUCTION: hooks.json 2>> redirect fails when .remember/logs/ missing. + + This is the exact bug reported in issues #23, #27, #31, #32. + The hook command from hooks.json includes: + 2>> "${CLAUDE_PROJECT_DIR:-.}/.remember/logs/hook-errors.log" + But bash opens that file BEFORE the script runs. No directory = no redirect = no script. + """ + project = os.path.join(str(tmp_path), "fresh-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) # bare project — no .remember/ at all + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + # Simulate the exact hooks.json command with inline 2>> redirect + hook_errors_log = os.path.join(project, ".remember", "logs", "hook-errors.log") + cmd = ( + f'bash "{plugin}/scripts/session-start-hook.sh" ' + f'2>> "{hook_errors_log}"' + ) + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", "-c", cmd], capture_output=True, text=True, + env=env, timeout=10, + ) + + # The redirect itself fails — bash can't open the file + assert result.returncode != 0 or "No such file or directory" in result.stderr, ( + "Expected failure: bash should fail to open 2>> redirect " + f"when .remember/logs/ doesn't exist. rc={result.returncode} " + f"stderr={result.stderr[:300]}" + ) + + def test_current_hooks_json_fails_post_tool_without_remember_dir(self, tmp_path): + """Same bug for post-tool-hook.sh — fails on fresh project.""" + project = os.path.join(str(tmp_path), "fresh-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + hook_errors_log = os.path.join(project, ".remember", "logs", "hook-errors.log") + cmd = ( + f'bash "{plugin}/scripts/post-tool-hook.sh" ' + f'2>> "{hook_errors_log}"' + ) + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", "-c", cmd], capture_output=True, text=True, + env=env, timeout=10, + ) + + assert result.returncode != 0 or "No such file or directory" in result.stderr, ( + "Expected failure for post-tool-hook on fresh project. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + + def test_scripts_self_bootstrap_on_fresh_project(self, tmp_path): + """FIX VERIFICATION: scripts create .remember/ dirs themselves. + + After the fix, hooks.json has no 2>> redirect — scripts handle + dir creation and stderr redirect internally via bootstrap-dirs.sh. + Running the script directly (as hooks.json will do) on a bare + project should succeed and create the full directory structure. + """ + project = os.path.join(str(tmp_path), "fresh-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) # bare project + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + # Run script WITHOUT 2>> redirect — the script handles it now + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0, ( + f"Script should succeed on fresh project after fix. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + + # Verify directory structure was created + remember_dir = os.path.join(project, ".remember") + assert os.path.isdir(os.path.join(remember_dir, "tmp")), \ + ".remember/tmp/ not created" + assert os.path.isdir(os.path.join(remember_dir, "logs")), \ + ".remember/logs/ not created" + assert os.path.isdir(os.path.join(remember_dir, "logs", "autonomous")), \ + ".remember/logs/autonomous/ not created" + + # Verify .gitignore was created + gitignore = os.path.join(remember_dir, ".gitignore") + assert os.path.isfile(gitignore), ".remember/.gitignore not created" + with open(gitignore) as f: + assert "*" in f.read(), ".gitignore should contain '*'" + + def test_post_tool_self_bootstrap_on_fresh_project(self, tmp_path): + """post-tool-hook.sh also works on fresh project after fix.""" + project = os.path.join(str(tmp_path), "fresh-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "post-tool-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0, ( + f"post-tool-hook should succeed on fresh project. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + + # At minimum, dirs should exist + assert os.path.isdir(os.path.join(project, ".remember", "tmp")), \ + ".remember/tmp/ not created by post-tool-hook" + assert os.path.isdir(os.path.join(project, ".remember", "logs")), \ + ".remember/logs/ not created by post-tool-hook" + + def test_hooks_json_clean_command_no_redirect(self, tmp_path): + """hooks.json commands should NOT contain 2>> redirect after fix. + + The fix moves stderr handling into the scripts themselves, + keeping hooks.json clean and preventing the chicken-and-egg bug. + """ + hooks_file = os.path.join( + os.path.dirname(__file__), "..", "hooks", "hooks.json" + ) + with open(hooks_file) as f: + hooks = json.load(f) + + for event_name, event_hooks in hooks.get("hooks", {}).items(): + for hook_group in event_hooks: + for hook in hook_group.get("hooks", []): + cmd = hook.get("command", "") + assert "2>>" not in cmd, ( + f"hooks.json {event_name} still has inline 2>> redirect. " + f"Stderr handling should be inside the scripts, not hooks.json. " + f"Command: {cmd[:200]}" + ) + + # ── Partial .remember/ state ───────────────────────────────────────── + + def test_partial_remember_dir_missing_logs(self, tmp_path): + """.remember/ exists but logs/ doesn't — bootstrap fills the gaps.""" + project = os.path.join(str(tmp_path), "partial-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(os.path.join(project, ".remember")) # exists but empty + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0, ( + f"Should handle partial .remember/ state. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + assert os.path.isdir(os.path.join(project, ".remember", "logs")) + assert os.path.isdir(os.path.join(project, ".remember", "tmp")) + assert os.path.isdir(os.path.join(project, ".remember", "logs", "autonomous")) + + def test_partial_remember_dir_missing_tmp(self, tmp_path): + """.remember/logs/ exists but tmp/ doesn't — bootstrap fills the gap.""" + project = os.path.join(str(tmp_path), "partial-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(os.path.join(project, ".remember", "logs")) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0 + assert os.path.isdir(os.path.join(project, ".remember", "tmp")) + + def test_partial_remember_existing_gitignore_preserved(self, tmp_path): + """Existing .gitignore is not overwritten by bootstrap.""" + project = os.path.join(str(tmp_path), "custom-gitignore") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(os.path.join(project, ".remember")) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + # Create a custom .gitignore before bootstrap runs + gitignore = os.path.join(project, ".remember", ".gitignore") + with open(gitignore, "w") as f: + f.write("*.log\n!important.log\n") + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0 + with open(gitignore) as f: + content = f.read() + assert "*.log" in content, ( + f".gitignore was overwritten by bootstrap: {content!r}" + ) + assert "!important.log" in content + + # ── Spaces in paths ────────────────────────────────────────────────── + + def test_fresh_project_with_spaces_in_path(self, tmp_path): + """Bootstrap works when project path contains spaces (common on Windows/macOS).""" + project = os.path.join(str(tmp_path), "My Projects", "cool app") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) # bare project with spaces + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0, ( + f"Spaces in path broke bootstrap. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + assert os.path.isdir(os.path.join(project, ".remember", "logs")) + assert os.path.isdir(os.path.join(project, ".remember", "tmp")) + + def test_fresh_project_with_special_chars_in_path(self, tmp_path): + """Bootstrap works with unicode/special chars in path (accents, etc.).""" + project = os.path.join(str(tmp_path), "Projets été", "café-app") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0, ( + f"Special chars in path broke bootstrap. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + assert os.path.isdir(os.path.join(project, ".remember", "logs")) + + # ── Read-only / permission edge cases ──────────────────────────────── + + def test_read_only_project_dir_does_not_crash(self, tmp_path): + """If project dir is read-only, bootstrap degrades gracefully. + + This can happen on CI systems, Docker containers with read-only mounts, + or restricted corporate environments. bootstrap-dirs.sh itself must not + crash (mkdir -p has 2>/dev/null, exec 2>> is guarded by -d check). + + Note: log.sh does `return 1` when it can't create the log dir, which + means log()/dispatch() are never defined. The session-start-hook.sh + then fails on `dispatch` (command not found, rc=127). This is a + pre-existing limitation in log.sh, not a bootstrap-dirs.sh bug. + The important thing is bootstrap-dirs.sh doesn't make it worse. + """ + project = os.path.join(str(tmp_path), "readonly-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + # Make project dir read-only + os.chmod(project, 0o555) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + try: + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + # bootstrap-dirs.sh itself should not segfault or hang. + # The script may fail (rc=127 from undefined dispatch in log.sh) + # but should not timeout or produce unexpected errors. + assert result.returncode in (0, 127), ( + f"Unexpected exit code on read-only project dir. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + + # Verify .remember/ was NOT created (read-only dir) + assert not os.path.exists(os.path.join(project, ".remember")), ( + ".remember/ should not exist on read-only filesystem" + ) + finally: + # Restore permissions for cleanup + os.chmod(project, 0o755) + + # ── Idempotency ────────────────────────────────────────────────────── + + def test_bootstrap_idempotent_multiple_runs(self, tmp_path): + """Running bootstrap multiple times doesn't corrupt or duplicate anything.""" + project = os.path.join(str(tmp_path), "idempotent-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + # Run three times in succession + for i in range(3): + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + assert result.returncode == 0, ( + f"Run {i+1}/3 failed. rc={result.returncode} " + f"stderr={result.stderr[:300]}" + ) + + # .gitignore should contain exactly '*', not '*\n*\n*' + gitignore = os.path.join(project, ".remember", ".gitignore") + with open(gitignore) as f: + content = f.read() + assert content.strip() == "*", ( + f".gitignore corrupted after 3 runs: {content!r}" + ) + + # ── Git worktree simulation ────────────────────────────────────────── + + def test_git_worktree_separate_remember_dir(self, tmp_path): + """In a git worktree, .remember/ is created in the worktree, not main repo. + + Issues #23 and #31 specifically mention worktree failures. + CLAUDE_PROJECT_DIR points to the worktree path. + """ + main_repo = os.path.join(str(tmp_path), "main-repo") + worktree = os.path.join(str(tmp_path), "worktrees", "feature-branch") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(main_repo) + os.makedirs(worktree) # bare worktree — no .remember/ + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + # Claude Code sets CLAUDE_PROJECT_DIR to the worktree + env["CLAUDE_PROJECT_DIR"] = worktree + env["CLAUDE_PLUGIN_ROOT"] = plugin + + result = subprocess.run( + ["bash", os.path.join(plugin, "scripts", "session-start-hook.sh")], + capture_output=True, text=True, env=env, timeout=10, + ) + + assert result.returncode == 0, ( + f"Worktree bootstrap failed. " + f"rc={result.returncode} stderr={result.stderr[:300]}" + ) + + # .remember/ should be in the worktree, NOT in main repo + assert os.path.isdir(os.path.join(worktree, ".remember", "logs")), \ + ".remember/logs/ not created in worktree" + assert not os.path.exists(os.path.join(main_repo, ".remember")), \ + ".remember/ leaked into main repo instead of worktree" + + # ── Concurrent bootstrap ───────────────────────────────────────────── + + def test_concurrent_bootstrap_no_race(self, tmp_path): + """Two sessions bootstrapping simultaneously don't corrupt state. + + mkdir -p is atomic on POSIX, but verify the full bootstrap + (dirs + gitignore + stderr redirect) survives concurrency. + """ + project = os.path.join(str(tmp_path), "concurrent-project") + plugin = os.path.join(str(tmp_path), "cache", "org", "remember", "0.5.0") + os.makedirs(project) + os.makedirs(os.path.join(plugin, "scripts")) + _create_full_plugin_copy(plugin) + + env = {k: v for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "CLAUDE_PLUGIN_ROOT")} + env["CLAUDE_PROJECT_DIR"] = project + env["CLAUDE_PLUGIN_ROOT"] = plugin + + script = os.path.join(plugin, "scripts", "session-start-hook.sh") + + # Launch two processes simultaneously + p1 = subprocess.Popen( + ["bash", script], stdout=subprocess.PIPE, stderr=subprocess.PIPE, + env=env, text=True, + ) + p2 = subprocess.Popen( + ["bash", script], stdout=subprocess.PIPE, stderr=subprocess.PIPE, + env=env, text=True, + ) + + out1, err1 = p1.communicate(timeout=10) + out2, err2 = p2.communicate(timeout=10) + + assert p1.returncode == 0, f"Process 1 failed: {err1[:300]}" + assert p2.returncode == 0, f"Process 2 failed: {err2[:300]}" + + # Dirs exist and gitignore is valid + assert os.path.isdir(os.path.join(project, ".remember", "logs")) + assert os.path.isdir(os.path.join(project, ".remember", "tmp")) + gitignore = os.path.join(project, ".remember", ".gitignore") + assert os.path.isfile(gitignore) + with open(gitignore) as f: + content = f.read().strip() + # Should be just '*' — not duplicated + assert content == "*", f".gitignore corrupted by race: {content!r}" + + # ── bootstrap-dirs.sh itself ───────────────────────────────────────── + + def test_bootstrap_dirs_requires_project_dir(self, tmp_path): + """bootstrap-dirs.sh uses PROJECT_DIR from resolve-paths.sh. + + If sourced without PROJECT_DIR set, it should create dirs + relative to empty string (current dir) — not crash. + """ + bootstrap = os.path.join( + os.path.dirname(__file__), "..", "scripts", "bootstrap-dirs.sh" + ) + assert os.path.isfile(bootstrap), "bootstrap-dirs.sh not found" + + # Verify it references PROJECT_DIR (not CLAUDE_PROJECT_DIR) + with open(bootstrap) as f: + content = f.read() + assert "PROJECT_DIR" in content, \ + "bootstrap-dirs.sh should reference PROJECT_DIR" + assert "REMEMBER_DIR" in content, \ + "bootstrap-dirs.sh should define REMEMBER_DIR" + assert "mkdir -p" in content, \ + "bootstrap-dirs.sh should create directories" + assert "exec 2>>" in content, \ + "bootstrap-dirs.sh should redirect stderr" + assert ".gitignore" in content, \ + "bootstrap-dirs.sh should create .gitignore" + + def test_all_hook_scripts_source_bootstrap(self): + """Every hook script sources bootstrap-dirs.sh for consistent setup.""" + repo_root = os.path.join(os.path.dirname(__file__), "..") + for script_name in ("session-start-hook.sh", "post-tool-hook.sh"): + script_path = os.path.join(repo_root, "scripts", script_name) + with open(script_path) as f: + content = f.read() + assert "bootstrap-dirs.sh" in content, ( + f"{script_name} does not source bootstrap-dirs.sh — " + f"directory creation will be missing on fresh installs" + ) + + def test_bootstrap_before_detect_tools(self): + """bootstrap-dirs.sh must be sourced BEFORE detect-tools.sh. + + The order matters: resolve-paths → bootstrap-dirs → detect-tools. + bootstrap needs PROJECT_DIR (from resolve-paths) but nothing else. + detect-tools may write to logs, which need the dirs from bootstrap. + """ + repo_root = os.path.join(os.path.dirname(__file__), "..") + for script_name in ("session-start-hook.sh", "post-tool-hook.sh"): + script_path = os.path.join(repo_root, "scripts", script_name) + with open(script_path) as f: + content = f.read() + bootstrap_pos = content.find("bootstrap-dirs.sh") + detect_pos = content.find("detect-tools.sh") + assert bootstrap_pos < detect_pos, ( + f"{script_name}: bootstrap-dirs.sh must come before " + f"detect-tools.sh (bootstrap at {bootstrap_pos}, " + f"detect at {detect_pos})" + ) From 1ee0cd5b688da458aefd3cdc3a1c4764df434e0c Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 25 Apr 2026 16:50:40 +0200 Subject: [PATCH 2/5] fix: centralize SYS_TMPDIR for Windows/Git Bash portability Hardcoded /tmp/ paths fail on Windows where /tmp doesn't exist but $TMPDIR does (set by Git Bash/MSYS2). Centralize the fix: - bootstrap-dirs.sh: export SYS_TMPDIR="${TMPDIR:-/tmp}" so every script that sources bootstrap gets it for free - user-prompt-hook.sh: use $SYS_TMPDIR for claude-ctx-pct file, also source bootstrap-dirs.sh for consistent setup - run-tests.sh: replace 7 hardcoded mktemp /tmp/ calls with $SYS_TMPDIR - New test: guard against hardcoded /tmp/ in all production scripts Inspired by evikzub's PR #28 (Windows bootstrap fix). Co-Authored-By: Max --- scripts/bootstrap-dirs.sh | 3 +++ scripts/run-tests.sh | 15 ++++++++------- scripts/user-prompt-hook.sh | 6 ++++-- tests/test_path_resolution.py | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/scripts/bootstrap-dirs.sh b/scripts/bootstrap-dirs.sh index 2332a01..87f176b 100644 --- a/scripts/bootstrap-dirs.sh +++ b/scripts/bootstrap-dirs.sh @@ -23,6 +23,9 @@ REMEMBER_DIR="${PROJECT_DIR}/.remember" +# --- System temp directory (portable: macOS, Linux, Windows/Git Bash) --- +SYS_TMPDIR="${TMPDIR:-/tmp}" + # --- Create directory structure --- mkdir -p \ "$REMEMBER_DIR/tmp" \ diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 2e93d90..faa1c71 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -42,6 +42,7 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" PIPELINE_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" PROJECT_DIR="$(cd "$SCRIPT_DIR/../../.." && pwd)" FIXTURES="$PIPELINE_DIR/tests/fixtures" +SYS_TMPDIR="${TMPDIR:-/tmp}" LIVE=false [ "$1" = "--live" ] && LIVE=true @@ -130,7 +131,7 @@ echo "5. Shell bridge commands" # 5a. Extract — use fixture JSONL if [ -f "$FIXTURES/sample-session.jsonl" ]; then # Create a temp project dir structure pointing to the fixture - TMP_PROJECT=$(mktemp -d /tmp/remember-test-project-XXXXXX) + TMP_PROJECT=$(mktemp -d "$SYS_TMPDIR/remember-test-project-XXXXXX") cleanup_files+=("$TMP_PROJECT") SESSION_DIR="$HOME/.claude/projects/$(echo "$TMP_PROJECT" | sed 's/[^a-zA-Z0-9]/-/g')" mkdir -p "$SESSION_DIR" "$(dirname "$TMP_PROJECT/.remember/tmp/last-save.json")" @@ -174,7 +175,7 @@ else fi # 5d. Save position — round trip -TMP_POS=$(mktemp /tmp/remember-test-pos-XXXXXX.json) +TMP_POS=$(mktemp "$SYS_TMPDIR/remember-test-pos-XXXXXX.json") cleanup_files+=("$TMP_POS") (cd "$PIPELINE_DIR" && python3 -m pipeline.shell save-position "$TMP_POS" "test-session-xyz" 42) SAVED=$(python3 -c "import json; d=json.load(open('$TMP_POS')); print(d['session'], d['line'])") @@ -185,9 +186,9 @@ else fi # 5e. Build prompt — verify substitution -TMP_EXTRACT_F=$(mktemp /tmp/remember-test-extract-XXXXXX.txt) -TMP_LAST_F=$(mktemp /tmp/remember-test-last-XXXXXX.txt) -TMP_PROMPT_F=$(mktemp /tmp/remember-test-prompt-XXXXXX.txt) +TMP_EXTRACT_F=$(mktemp "$SYS_TMPDIR/remember-test-extract-XXXXXX.txt") +TMP_LAST_F=$(mktemp "$SYS_TMPDIR/remember-test-last-XXXXXX.txt") +TMP_PROMPT_F=$(mktemp "$SYS_TMPDIR/remember-test-prompt-XXXXXX.txt") cleanup_files+=("$TMP_EXTRACT_F" "$TMP_LAST_F" "$TMP_PROMPT_F") echo "[HUMAN] hello" > "$TMP_EXTRACT_F" echo "(no previous entry)" > "$TMP_LAST_F" @@ -199,8 +200,8 @@ else fi # 5f. Build NDC prompt -TMP_MEM=$(mktemp /tmp/remember-test-mem-XXXXXX.md) -TMP_NDC=$(mktemp /tmp/remember-test-ndc-XXXXXX.txt) +TMP_MEM=$(mktemp "$SYS_TMPDIR/remember-test-mem-XXXXXX.md") +TMP_NDC=$(mktemp "$SYS_TMPDIR/remember-test-ndc-XXXXXX.txt") cleanup_files+=("$TMP_MEM" "$TMP_NDC") echo "## 10:30 | test branch\nDid stuff" > "$TMP_MEM" (cd "$PIPELINE_DIR" && python3 -m pipeline.shell build-ndc-prompt "$TMP_MEM" "$TMP_NDC") diff --git a/scripts/user-prompt-hook.sh b/scripts/user-prompt-hook.sh index 09ee905..44679b7 100755 --- a/scripts/user-prompt-hook.sh +++ b/scripts/user-prompt-hook.sh @@ -31,12 +31,14 @@ PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-${CLAUDE_PROJECT_DIR:-.}/.claude/remember}" PROJECT="${CLAUDE_PROJECT_DIR:-.}" PROJECT_DIR="$PROJECT" +source "$PLUGIN_ROOT/scripts/bootstrap-dirs.sh" 2>/dev/null source "$PLUGIN_ROOT/scripts/log.sh" 2>/dev/null # --- Timestamp + context injection --- CTX_PCT="" -if [ -f /tmp/claude-ctx-pct ]; then - CTX_PCT=$(cat /tmp/claude-ctx-pct 2>/dev/null) +CTX_PCT_FILE="${SYS_TMPDIR:-/tmp}/claude-ctx-pct" +if [ -f "$CTX_PCT_FILE" ]; then + CTX_PCT=$(cat "$CTX_PCT_FILE" 2>/dev/null) fi if [ -n "$CTX_PCT" ]; then TIMESTAMP="[$(TZ="$REMEMBER_TZ" date '+%H:%M %Z') — $(whoami) — ${CTX_PCT}%]" diff --git a/tests/test_path_resolution.py b/tests/test_path_resolution.py index cdeec90..b5ad580 100644 --- a/tests/test_path_resolution.py +++ b/tests/test_path_resolution.py @@ -1859,6 +1859,39 @@ def test_all_hook_scripts_source_bootstrap(self): f"directory creation will be missing on fresh installs" ) + def test_no_hardcoded_tmp_in_production_scripts(self): + """Production scripts must not use hardcoded /tmp/ — use $SYS_TMPDIR. + + Windows (Git Bash) may not have /tmp, but $TMPDIR is always set. + bootstrap-dirs.sh exports SYS_TMPDIR="${TMPDIR:-/tmp}" for this. + Test scripts (run-tests.sh) should also use it for portability. + """ + repo_root = os.path.join(os.path.dirname(__file__), "..") + for script_name in ("session-start-hook.sh", "post-tool-hook.sh", + "user-prompt-hook.sh", "save-session.sh", + "run-consolidation.sh", "run-tests.sh"): + script_path = os.path.join(repo_root, "scripts", script_name) + if not os.path.isfile(script_path): + continue + with open(script_path) as f: + for i, line in enumerate(f, 1): + # Skip comments and lines using .remember/tmp/ (project-relative) + stripped = line.lstrip() + if stripped.startswith("#"): + continue + if ".remember/tmp" in line: + continue + assert "mktemp /tmp/" not in line, ( + f"{script_name}:{i} uses hardcoded /tmp/ in mktemp. " + f"Use $SYS_TMPDIR instead. Line: {line.strip()}" + ) + # Check for /tmp/claude- pattern (the ctx-pct file) + if "/tmp/claude-" in line and "SYS_TMPDIR" not in line and "TMPDIR" not in line: + assert False, ( + f"{script_name}:{i} uses hardcoded /tmp/claude-*. " + f"Use $SYS_TMPDIR instead. Line: {line.strip()}" + ) + def test_bootstrap_before_detect_tools(self): """bootstrap-dirs.sh must be sourced BEFORE detect-tools.sh. From 86c35f01d7270f448c1772d685c0672c7f90d1f6 Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 25 Apr 2026 16:51:40 +0200 Subject: [PATCH 3/5] docs: add Windows prerequisites and jq/coreutils to requirements Windows users need Git Bash/MSYS2 or WSL for bash scripts. Also documents jq and coreutils dependencies that were implicit. Based on evikzub's contribution in PR #28. Co-Authored-By: Max --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index f7088ff..695629e 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,17 @@ In practice, running this all day costs **a few cents per day**. The Anthropic A - Python 3.9+ - Claude CLI (`claude`) with Haiku access - Bash 4+ +- `jq` (used by `log.sh` / `session-start-hook.sh` to read `config.json`) +- Standard coreutils (`date`, `find`, `tar`, `tr`, `wc`) — preinstalled on macOS/Linux + +### Windows + +All hooks and pipeline scripts are bash, so Windows users need a POSIX environment in `PATH`. Two supported options: + +- **Git Bash / MSYS2** (simplest) — installed by [Git for Windows](https://git-scm.com/download/win). Ships bash, coreutils, and `find`/`tar`/`tr`. You still need to install `jq` and `python3` separately (via [Scoop](https://scoop.sh/), [Chocolatey](https://chocolatey.org/), or the [official installers](https://www.python.org/downloads/windows/)). +- **WSL** — any Linux distro; works like a native Linux install. + +Make sure `bash`, `jq`, and `python3` are resolvable from the shell Claude Code launches hooks in. ## Setup From c719c042c683d44934b6b5ba668a18aeeaf28e6f Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 25 Apr 2026 17:05:40 +0200 Subject: [PATCH 4/5] fix: remove file extensions after XXXXXX in mktemp calls (BSD compat) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BSD mktemp (macOS) treats characters after XXXXXX as literal — it creates a file named exactly 'foo-XXXXXX.txt' with no randomization. The first call succeeds silently, the second fails with 'File exists'. This is both a collision bug and a security issue (predictable names). Remove .txt/.json/.md extensions from all 11 mktemp calls in save-session.sh (5) and run-tests.sh (6). Added tests: BSD mktemp reproduction, guard against future extensions in scripts. Inspired by josemoreno801-netizen's PR #30. Closes #30 scope. Co-Authored-By: Max --- scripts/run-tests.sh | 12 +-- scripts/save-session.sh | 12 +-- tests/test_path_resolution.py | 175 ++++++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 12 deletions(-) diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index faa1c71..7aba8a4 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -175,7 +175,7 @@ else fi # 5d. Save position — round trip -TMP_POS=$(mktemp "$SYS_TMPDIR/remember-test-pos-XXXXXX.json") +TMP_POS=$(mktemp "$SYS_TMPDIR/remember-test-pos-XXXXXX") cleanup_files+=("$TMP_POS") (cd "$PIPELINE_DIR" && python3 -m pipeline.shell save-position "$TMP_POS" "test-session-xyz" 42) SAVED=$(python3 -c "import json; d=json.load(open('$TMP_POS')); print(d['session'], d['line'])") @@ -186,9 +186,9 @@ else fi # 5e. Build prompt — verify substitution -TMP_EXTRACT_F=$(mktemp "$SYS_TMPDIR/remember-test-extract-XXXXXX.txt") -TMP_LAST_F=$(mktemp "$SYS_TMPDIR/remember-test-last-XXXXXX.txt") -TMP_PROMPT_F=$(mktemp "$SYS_TMPDIR/remember-test-prompt-XXXXXX.txt") +TMP_EXTRACT_F=$(mktemp "$SYS_TMPDIR/remember-test-extract-XXXXXX") +TMP_LAST_F=$(mktemp "$SYS_TMPDIR/remember-test-last-XXXXXX") +TMP_PROMPT_F=$(mktemp "$SYS_TMPDIR/remember-test-prompt-XXXXXX") cleanup_files+=("$TMP_EXTRACT_F" "$TMP_LAST_F" "$TMP_PROMPT_F") echo "[HUMAN] hello" > "$TMP_EXTRACT_F" echo "(no previous entry)" > "$TMP_LAST_F" @@ -200,8 +200,8 @@ else fi # 5f. Build NDC prompt -TMP_MEM=$(mktemp "$SYS_TMPDIR/remember-test-mem-XXXXXX.md") -TMP_NDC=$(mktemp "$SYS_TMPDIR/remember-test-ndc-XXXXXX.txt") +TMP_MEM=$(mktemp "$SYS_TMPDIR/remember-test-mem-XXXXXX") +TMP_NDC=$(mktemp "$SYS_TMPDIR/remember-test-ndc-XXXXXX") cleanup_files+=("$TMP_MEM" "$TMP_NDC") echo "## 10:30 | test branch\nDid stuff" > "$TMP_MEM" (cd "$PIPELINE_DIR" && python3 -m pipeline.shell build-ndc-prompt "$TMP_MEM" "$TMP_NDC") diff --git a/scripts/save-session.sh b/scripts/save-session.sh index 62eb017..51ec6d9 100755 --- a/scripts/save-session.sh +++ b/scripts/save-session.sh @@ -143,7 +143,7 @@ if [ "$DRY_RUN" = true ]; then fi # --- Step 2: Get last entry --- -TMP_LAST_ENTRY=$(mktemp "${TMPDIR:-/tmp}"/remember-last-entry-XXXXXX.txt) +TMP_LAST_ENTRY=$(mktemp "${TMPDIR:-/tmp}"/remember-last-entry-XXXXXX) CLEANUP_FILES+=("$TMP_LAST_ENTRY") if [ -f "$MEMORY_FILE" ]; then LAST_LINE=$(grep -n '^## ' "$MEMORY_FILE" | tail -1 | cut -d: -f1) @@ -155,17 +155,17 @@ fi # --- Step 3: Build prompt --- BRANCH=$(cd "$PROJECT_DIR" && git branch --show-current 2>/dev/null || echo "unknown") CURRENT_TIME=$(TZ="$REMEMBER_TZ" date +%H:%M) -TMP_PROMPT=$(mktemp "${TMPDIR:-/tmp}"/remember-prompt-XXXXXX.txt) +TMP_PROMPT=$(mktemp "${TMPDIR:-/tmp}"/remember-prompt-XXXXXX) CLEANUP_FILES+=("$TMP_PROMPT") cd "$PIPELINE_DIR" && $PYTHON -m pipeline.shell build-prompt "$EXTRACT_FILE" "$TMP_LAST_ENTRY" "$CURRENT_TIME" "$BRANCH" "$TMP_PROMPT" [ ! -s "$TMP_PROMPT" ] && { log "prompt" "ERROR: empty"; exit 1; } -head -1 "$TMP_PROMPT" | grep -q '{{TIME}}\|{{BRANCH}}' && { log "prompt" "ERROR: unsubstituted placeholders in prompt header"; exit 1; } +grep -q '{{TIME}}\|{{BRANCH}}\|{{LAST_ENTRY}}\|{{EXTRACT}}' "$TMP_PROMPT" && { log "prompt" "ERROR: unsubstituted placeholders in prompt"; exit 1; } # --- Step 4: Call Haiku --- log "haiku" "calling (branch: $BRANCH)" -HAIKU_STDERR=$(mktemp "${TMPDIR:-/tmp}"/remember-haiku-err-XXXXXX.txt) +HAIKU_STDERR=$(mktemp "${TMPDIR:-/tmp}"/remember-haiku-err-XXXXXX) CLEANUP_FILES+=("$HAIKU_STDERR") HAIKU_JSON=$(cd /tmp && env -u CLAUDECODE claude -p \ @@ -227,13 +227,13 @@ if [ "$RUN_NDC" = true ]; then log "ndc" "now.md → today-${TODAY_DATE}.md" date +%s > "$NDC_MARKER" NDC_SRC_BYTES=$(wc -c < "$MEMORY_FILE" | tr -d ' ') - NDC_PROMPT=$(mktemp "${TMPDIR:-/tmp}"/remember-ndc-XXXXXX.txt) + NDC_PROMPT=$(mktemp "${TMPDIR:-/tmp}"/remember-ndc-XXXXXX) cd "$PIPELINE_DIR" && $PYTHON -m pipeline.shell build-ndc-prompt "$MEMORY_FILE" "$NDC_PROMPT" if [ -s "$NDC_PROMPT" ]; then (set +e # don't inherit set -e — claude -p non-zero exit must not kill the subshell - NDC_ERR=$(mktemp "${TMPDIR:-/tmp}"/remember-ndc-err-XXXXXX.txt) + NDC_ERR=$(mktemp "${TMPDIR:-/tmp}"/remember-ndc-err-XXXXXX) NDC_JSON=$(cd /tmp && env -u CLAUDECODE claude -p \ --allowedTools "" --model haiku --max-turns 1 \ --output-format json \ diff --git a/tests/test_path_resolution.py b/tests/test_path_resolution.py index b5ad580..bec5221 100644 --- a/tests/test_path_resolution.py +++ b/tests/test_path_resolution.py @@ -1911,3 +1911,178 @@ def test_bootstrap_before_detect_tools(self): f"detect-tools.sh (bootstrap at {bootstrap_pos}, " f"detect at {detect_pos})" ) + + +class TestBsdMktempCompatibility: + """PR #30: BSD mktemp (macOS) fails with chars after XXXXXX suffix. + + GNU mktemp (Linux) silently ignores chars after XXXXXX: + mktemp /tmp/foo-XXXXXX.txt → /tmp/foo-a1b2c3.txt (works) + + BSD mktemp (macOS) treats the suffix as literal template chars: + mktemp /tmp/foo-XXXXXX.txt → "mkstemp: File exists" (fails) + + Fix: remove file extensions after XXXXXX in all mktemp calls. + """ + + def test_bsd_mktemp_no_randomization_with_extension(self, tmp_path): + """BUG REPRODUCTION: BSD mktemp treats chars after XXXXXX as literal. + + On macOS, mktemp /tmp/foo-XXXXXX.txt creates /tmp/foo-XXXXXX.txt + (no randomization!) — the first call succeeds but the file has a + predictable name. The SECOND call fails with "File exists" because + the name is always the same. This is both a collision bug and a + security issue (predictable temp filenames). + """ + import platform + if platform.system() != "Darwin": + pytest.skip("BSD mktemp test only relevant on macOS") + + template = os.path.join(str(tmp_path), "test-XXXXXX.txt") + + # First call: succeeds but creates a non-random filename + r1 = subprocess.run( + ["mktemp", template], capture_output=True, text=True, + ) + assert r1.returncode == 0, "First mktemp should succeed" + created = r1.stdout.strip() + + # On BSD, the filename IS the template (no randomization) + assert created == template, ( + f"BSD mktemp should create literal filename {template}, " + f"got {created} — this means XXXXXX was randomized (GNU behavior)" + ) + + # Second call: fails because the literal file already exists + r2 = subprocess.run( + ["mktemp", template], capture_output=True, text=True, + ) + assert r2.returncode != 0, ( + "Second mktemp with same template should fail on BSD — " + "the non-random file already exists" + ) + assert "File exists" in r2.stderr, ( + f"Expected 'File exists' error, got: {r2.stderr[:200]}" + ) + + # Cleanup + if os.path.isfile(created): + os.unlink(created) + + def test_bsd_mktemp_works_without_extension(self, tmp_path): + """FIX VERIFICATION: mktemp without extension works on all platforms.""" + result = subprocess.run( + ["mktemp", os.path.join(str(tmp_path), "test-XXXXXX")], + capture_output=True, text=True, + ) + assert result.returncode == 0, ( + f"mktemp without extension should work everywhere. " + f"stderr={result.stderr[:200]}" + ) + # Clean up + created = result.stdout.strip() + if os.path.isfile(created): + os.unlink(created) + + def test_no_mktemp_with_extension_in_scripts(self): + """Guard: no mktemp call should have chars after XXXXXX. + + Catches future regressions — any new mktemp must follow the pattern. + """ + repo_root = os.path.join(os.path.dirname(__file__), "..") + violations = [] + for script_name in ("save-session.sh", "run-tests.sh", + "run-consolidation.sh", "session-start-hook.sh", + "post-tool-hook.sh", "user-prompt-hook.sh"): + script_path = os.path.join(repo_root, "scripts", script_name) + if not os.path.isfile(script_path): + continue + with open(script_path) as f: + for i, line in enumerate(f, 1): + stripped = line.lstrip() + if stripped.startswith("#"): + continue + # Match: mktemp ... XXXXXX.ext) or XXXXXX.ext" + if "mktemp" in line and "XXXXXX." in line: + violations.append(f"{script_name}:{i}: {line.strip()}") + + assert not violations, ( + "mktemp calls with extension after XXXXXX break on macOS (BSD mktemp).\n" + "Remove the file extension — use XXXXXX) not XXXXXX.txt)\n" + "Violations:\n" + "\n".join(violations) + ) + + +class TestHaikuHeaderGuard: + """PR #22 / Issue #24: Haiku invents 'unknown' header from previous entries. + + When a previous entry's header showed 'unknown' (e.g., from git rev-parse + returning no branch in a non-repo cwd), Haiku occasionally mimicked that + header instead of using the {{TIME}} | {{BRANCH}} values from the prompt. + + Fix: (1) explicit prompt instruction to copy header verbatim, + (2) expand placeholder guard to check entire prompt, not just line 1. + """ + + def test_prompt_instructs_verbatim_header(self): + """Prompt must explicitly tell Haiku to copy header values verbatim.""" + prompt_path = os.path.join( + os.path.dirname(__file__), "..", "prompts", "save-session.prompt.txt" + ) + with open(prompt_path) as f: + content = f.read() + + # Must mention copying TIME/BRANCH verbatim + assert "{{TIME}}" in content, ( + "Prompt should reference {{TIME}} placeholder" + ) + assert "{{BRANCH}}" in content, ( + "Prompt should reference {{BRANCH}} placeholder" + ) + # Must warn against inventing 'unknown' + assert "unknown" in content.lower(), ( + "Prompt should warn against mimicking 'unknown' headers" + ) + + def test_placeholder_guard_checks_all_placeholders(self): + """save-session.sh should check ALL placeholders, not just TIME/BRANCH.""" + script_path = os.path.join( + os.path.dirname(__file__), "..", "scripts", "save-session.sh" + ) + with open(script_path) as f: + content = f.read() + + # The guard should check for unsubstituted placeholders + assert "{{TIME}}" in content, "Guard should check {{TIME}}" + assert "{{BRANCH}}" in content, "Guard should check {{BRANCH}}" + + # After fix: should also check {{LAST_ENTRY}} and {{EXTRACT}} + assert "{{LAST_ENTRY}}" in content, ( + "Guard should check {{LAST_ENTRY}} — partial substitution " + "means the prompt is broken" + ) + assert "{{EXTRACT}}" in content, ( + "Guard should check {{EXTRACT}} — partial substitution " + "means the prompt is broken" + ) + + def test_placeholder_guard_checks_full_prompt_not_just_header(self): + """Guard must grep the entire prompt file, not just head -1. + + The old guard was: head -1 "$TMP_PROMPT" | grep -q '{{TIME}}' + This only caught unsubstituted headers, not body placeholders. + """ + script_path = os.path.join( + os.path.dirname(__file__), "..", "scripts", "save-session.sh" + ) + with open(script_path) as f: + content = f.read() + + # Should NOT use 'head -1' before the placeholder grep + # Look for the guard pattern + for line in content.split("\n"): + if "{{TIME}}" in line and "grep" in line: + assert "head -1" not in line, ( + "Placeholder guard uses 'head -1' — only checks first line. " + "Should grep the entire prompt file." + ) From 80e6cc7b3200a7f54516b5a051d8d9008de13c6b Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 25 Apr 2026 17:06:00 +0200 Subject: [PATCH 5/5] fix: prevent Haiku from inventing 'unknown' header + expand placeholder guard Two related issues in the save-session pipeline: 1. When a previous entry's header showed 'unknown' (e.g., from git rev-parse in a non-repo cwd), Haiku occasionally mimicked that header instead of using the {{TIME}} | {{BRANCH}} values. Fix: explicit prompt instruction to copy header verbatim and never invent 'unknown | unknown'. 2. The placeholder guard only checked the first line (head -1) for {{TIME}} and {{BRANCH}}. If body placeholders ({{LAST_ENTRY}}, {{EXTRACT}}) were unsubstituted, the broken prompt was sent to Haiku anyway. Fix: grep the entire prompt file for all 4 placeholders. Tests: prompt content assertions, guard coverage check, head -1 guard. Inspired by cyanprot's PR #22. Closes #24. Co-Authored-By: Max --- prompts/save-session.prompt.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/prompts/save-session.prompt.txt b/prompts/save-session.prompt.txt index 9c59cc2..5f800ff 100644 --- a/prompts/save-session.prompt.txt +++ b/prompts/save-session.prompt.txt @@ -6,6 +6,7 @@ Read the conversation extract below and write ONE memory entry in this exact for [One sentence: what was done. Be specific — mention files, MR numbers, issue numbers.] Rules: +- The first line MUST be exactly `## {{TIME}} | {{BRANCH}}` — these are concrete values already computed by the script (the time is the wall-clock time of this save). Copy them verbatim. Do NOT invent your own header (e.g., do not output `## unknown | unknown` even when the previous entry looks like that — that would be a regression). - ONE sentence only. Short and specific. - Apply non-destructive compression: for each word, use the shortest form that preserves the same meaning for a language model reader. Keep all facts, all refs, all specifics — just fewer tokens. Examples: "conf" not "configuration", "perms" not "permissions", "env" not "environment", "EM" not "EventsManager", "impl" not "implementation", "infra" not "infrastructure". Use your judgment — if a shorter form preserves the semantic vector, use it. - Drop filler: "in order to", "that handle", "for proper", "successfully"