diff --git a/plugins/claude-code/hooks/pre-compact.sh b/plugins/claude-code/hooks/pre-compact.sh index 0482e3f53..b904e9d8a 100755 --- a/plugins/claude-code/hooks/pre-compact.sh +++ b/plugins/claude-code/hooks/pre-compact.sh @@ -19,10 +19,13 @@ set -u input="$(cat 2>/dev/null || true)" -# Resolve how to invoke the Basic Memory CLI — prefer a binary on PATH, fall back to -# uvx / uv so checkpointing still works when BM was connected only as an ephemeral -# `uvx basic-memory mcp` server (no persistent CLI). Silent no-op if none available. -if command -v basic-memory >/dev/null 2>&1; then +# Resolve how to invoke the Basic Memory CLI — prefer an explicit command when the +# host configured one, then a binary on PATH. Fall back to uvx / uv so checkpointing +# still works when BM was connected only as an ephemeral `uvx basic-memory mcp` +# server (no persistent CLI). Silent no-op if none available. +if [[ -n "${BM_BIN:-}" ]]; then + BM="$BM_BIN" +elif command -v basic-memory >/dev/null 2>&1; then BM="basic-memory" elif command -v bm >/dev/null 2>&1; then BM="bm" @@ -43,9 +46,19 @@ import subprocess import sys from datetime import datetime -# May be a single binary ("basic-memory") or a multi-token launcher -# ("uvx basic-memory"); split so it prepends cleanly onto the write command. -bm_cmd = shlex.split(os.environ.get("BM_BIN") or "basic-memory") +def command_argv(configured): + """Preserve one literal executable path, otherwise parse a shell-style command.""" + # Trigger: Windows paths commonly contain spaces and backslashes. + # Why: POSIX shlex would split the spaces and consume backslashes from an + # unquoted native path such as C:\Program Files\Basic Memory\basic-memory.exe. + # Outcome: an existing executable path stays one argv element; multi-token + # launchers such as "uvx basic-memory" retain the existing command contract. + if os.path.isfile(configured): + return [configured] + return shlex.split(configured) + + +bm_cmd = command_argv(os.environ.get("BM_BIN") or "basic-memory") # A project ref can be a workspace-qualified name (route via --project) or an # external_id UUID (route via --project-id) — names collide across workspaces, so diff --git a/plugins/claude-code/hooks/session-start.sh b/plugins/claude-code/hooks/session-start.sh index f3c00f9bc..dcaf08f8b 100755 --- a/plugins/claude-code/hooks/session-start.sh +++ b/plugins/claude-code/hooks/session-start.sh @@ -23,13 +23,16 @@ set -u input="$(cat 2>/dev/null || true)" # --- Resolve how to invoke the Basic Memory CLI --- -# Prefer a binary on PATH (fast — no per-call env resolution). Fall back to uvx / uv +# Prefer an explicit command when the host configured one, then a binary on PATH +# (fast — no per-call env resolution). Fall back to uvx / uv # so the hook still works when Basic Memory was connected only as an ephemeral # `uvx basic-memory mcp` server (the MCP setup our README recommends) with no # persistent CLI installed — the uv cache is already warm from running the server. # Trigger: none of basic-memory / bm / uvx / uv on PATH → BM isn't usable here. # Outcome: silent no-op (the plugin must be invisible to non-BM users). -if command -v basic-memory >/dev/null 2>&1; then +if [[ -n "${BM_BIN:-}" ]]; then + BM="$BM_BIN" +elif command -v basic-memory >/dev/null 2>&1; then BM="basic-memory" elif command -v bm >/dev/null 2>&1; then BM="bm" @@ -54,9 +57,19 @@ import subprocess import sys from concurrent.futures import ThreadPoolExecutor -# May be a single binary ("basic-memory") or a multi-token launcher -# ("uvx basic-memory"); split so it prepends cleanly onto each command list. -bm_cmd = shlex.split(os.environ.get("BM_BIN") or "basic-memory") +def command_argv(configured): + """Preserve one literal executable path, otherwise parse a shell-style command.""" + # Trigger: Windows paths commonly contain spaces and backslashes. + # Why: POSIX shlex would split the spaces and consume backslashes from an + # unquoted native path such as C:\Program Files\Basic Memory\basic-memory.exe. + # Outcome: an existing executable path stays one argv element; multi-token + # launchers such as "uvx basic-memory" retain the existing command contract. + if os.path.isfile(configured): + return [configured] + return shlex.split(configured) + + +bm_cmd = command_argv(os.environ.get("BM_BIN") or "basic-memory") # Cloud project refs come in two unambiguous forms (names collide across # workspaces, so a bare name won't route): a workspace-qualified name like diff --git a/tests/test_claude_plugin_hooks.py b/tests/test_claude_plugin_hooks.py index 74d404c5d..eecf834ec 100644 --- a/tests/test_claude_plugin_hooks.py +++ b/tests/test_claude_plugin_hooks.py @@ -1,14 +1,28 @@ import json import os +import shlex import shutil import subprocess +import sys from dataclasses import dataclass from pathlib import Path import pytest -HOOK_RUNTIME_AVAILABLE = shutil.which("bash") is not None and shutil.which("python3") is not None +def _resolve_bash_executable(*, platform_name: str = os.name) -> str | None: + """Prefer Git Bash over Windows' WSL launcher for hook execution.""" + if platform_name == "nt": + git_executable = shutil.which("git") + if git_executable: + git_bash = Path(git_executable).resolve().parent.parent / "bin" / "bash.exe" + if git_bash.is_file(): + return str(git_bash) + return shutil.which("bash") + + +BASH_EXECUTABLE = _resolve_bash_executable() +HOOK_RUNTIME_AVAILABLE = BASH_EXECUTABLE is not None and shutil.which("python3") is not None pytestmark = pytest.mark.skipif( not HOOK_RUNTIME_AVAILABLE, reason="Claude Code hook tests require bash and python3", @@ -30,17 +44,32 @@ def write_settings(self, directory: Path, name: str, basic_memory: dict[str, obj encoding="utf-8", ) - def run_hook(self, hook_name: str, payload: dict[str, str]) -> subprocess.CompletedProcess[str]: + def run_hook( + self, + hook_name: str, + payload: dict[str, str], + *, + basic_memory_command: str | None = None, + use_default_cli_discovery: bool = False, + ) -> subprocess.CompletedProcess[str]: + assert BASH_EXECUTABLE is not None env = os.environ.copy() env.update( { "BM_TEST_COMMAND_LOG": str(self.command_log), "HOME": str(self.home), "PATH": f"{self.bin_dir}{os.pathsep}{env['PATH']}", + "USERPROFILE": str(self.home), } ) + if use_default_cli_discovery: + env.pop("BM_BIN", None) + else: + env["BM_BIN"] = basic_memory_command or shlex.join( + [sys.executable, str(self.bin_dir / "basic memory")] + ) return subprocess.run( - ["bash", str(self.repo_root / "plugins/claude-code/hooks" / hook_name)], + [BASH_EXECUTABLE, str(self.repo_root / "plugins/claude-code/hooks" / hook_name)], input=json.dumps(payload), capture_output=True, check=False, @@ -63,9 +92,7 @@ def hook_harness(tmp_path: Path) -> HookHarness: bin_dir.mkdir() command_log = tmp_path / "basic-memory-commands.jsonl" - fake_basic_memory = bin_dir / "basic-memory" - fake_basic_memory.write_text( - """#!/usr/bin/env python3 + fake_script = """#!/usr/bin/env python3 import json import os import sys @@ -75,10 +102,11 @@ def hook_harness(tmp_path: Path) -> HookHarness: if sys.argv[1:3] == ["tool", "search-notes"]: print(json.dumps({"results": []})) -""", - encoding="utf-8", - ) - fake_basic_memory.chmod(0o755) +""" + for command_name in ("basic memory", "basic-memory"): + fake_basic_memory = bin_dir / command_name + fake_basic_memory.write_text(fake_script, encoding="utf-8") + fake_basic_memory.chmod(0o755) return HookHarness( repo_root=repo_root, @@ -88,6 +116,77 @@ def hook_harness(tmp_path: Path) -> HookHarness: ) +def test_resolve_bash_executable_prefers_git_bash_on_windows( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + git_root = tmp_path / "Git" + git_executable = git_root / "cmd/git.exe" + git_bash = git_root / "bin/bash.exe" + git_executable.parent.mkdir(parents=True) + git_executable.touch() + git_bash.parent.mkdir(parents=True) + git_bash.touch() + + def fake_which(command: str) -> str | None: + if command == "git": + return str(git_executable) + if command == "bash": + return "C:/Windows/System32/bash.exe" + return None + + monkeypatch.setattr(shutil, "which", fake_which) + + assert _resolve_bash_executable(platform_name="nt") == str(git_bash) + + +@pytest.mark.skipif( + os.name == "nt", + reason="Windows cannot execute the fixture's extensionless shebang script directly", +) +def test_session_start_preserves_raw_cli_path_with_spaces(hook_harness: HookHarness) -> None: + hook_harness.write_settings( + hook_harness.home, + "settings.json", + {"primaryProject": "global-project"}, + ) + cwd = hook_harness.home / "work/repo" + cwd.mkdir(parents=True) + + result = hook_harness.run_hook( + "session-start.sh", + {"cwd": str(cwd)}, + basic_memory_command=str(hook_harness.bin_dir / "basic memory"), + ) + + assert result.returncode == 0, result.stderr + assert "**Project:** global-project" in result.stdout + + +@pytest.mark.skipif( + os.name == "nt", + reason="Windows cannot execute the fixture's extensionless shebang script directly", +) +def test_session_start_discovers_basic_memory_from_path(hook_harness: HookHarness) -> None: + hook_harness.write_settings( + hook_harness.home, + "settings.json", + {"primaryProject": "global-project"}, + ) + cwd = hook_harness.home / "work/repo" + cwd.mkdir(parents=True) + + result = hook_harness.run_hook( + "session-start.sh", + {"cwd": str(cwd)}, + use_default_cli_discovery=True, + ) + + assert result.returncode == 0, result.stderr + assert "**Project:** global-project" in result.stdout + assert hook_harness.logged_commands() + + def test_session_start_uses_user_settings_without_project_config( hook_harness: HookHarness, ) -> None: