diff --git a/CHANGELOG.md b/CHANGELOG.md index f25f176..d44e9ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [8.7.1] - 2026-08-15 + +### Fixed +- **Windows: the Codex provisioner now recognizes its own hooks (#261).** The + dedup/verify predicate substring-matched the literal `omind hook`, which the + Windows launcher form `omind.EXE hook ...` never contains — so setup + false-negatived its own SessionStart/PostToolUse entries and appended a + duplicate pair on every re-run. All Codex and Hermes marker checks now share + the Windows-tolerant `command_is_omind_hook` predicate from `hooks.py` + (previously private to the Claude provisioner). +- **A missing claude CLI degrades setup instead of refusing it (#258).** + `--dry-run` warned and previewed the full plan while the real run exited 1 + before doing anything. `claude` is now a soft prerequisite: the vault, seeds + and hooks still provision, MCP registration/verification skip with a clear + warning, and doctor flags the gap. Missing hard tools still abort. + ## [8.7.0] - 2026-08-15 ### Added diff --git a/pyproject.toml b/pyproject.toml index 4cc12bf..d0010f0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "omind" -version = "8.7.0" +version = "8.7.1" description = "Reproduce the OMI/Obsidian memory integration for AI agents, plus a local web app to view, edit, and add memory entries." readme = "README.md" requires-python = ">=3.10" diff --git a/src/omind/__init__.py b/src/omind/__init__.py index a4ba86b..799a170 100644 --- a/src/omind/__init__.py +++ b/src/omind/__init__.py @@ -2,4 +2,4 @@ # Copyright 2026 Aaron K. Clark """omind — OMI/Obsidian memory tooling for AI agents.""" -__version__ = "8.7.0" +__version__ = "8.7.1" diff --git a/src/omind/agents.py b/src/omind/agents.py index 68c044b..60f8999 100644 --- a/src/omind/agents.py +++ b/src/omind/agents.py @@ -34,7 +34,7 @@ import yaml from omind import paths, seeds -from omind.hooks import HOOK_MARKER +from omind.hooks import HOOK_MARKER, command_is_omind_hook from omind.provision import ( LEGACY_SERVER_NAME, CheckResult, @@ -191,10 +191,27 @@ def _codex_omind_hook(event: str, hook: dict[str, Any]) -> bool: if event in {"PreToolUse", "PermissionRequest"}: return CODEX_GUARD_MARKER in command if event in {"PostToolUse", "SessionStart"}: - return HOOK_MARKER in command + return command_is_omind_hook(command) return False +def _group_has_omind_hook(group: object) -> bool: + """True when a hooks-array matcher group contains an ``omind hook`` command. + + Checks the command strings themselves rather than substring-matching the + group's JSON dump: Windows resolves the executable to ``omind.EXE``, so the + dump never contains the literal :data:`HOOK_MARKER` and the old idiom both + failed verification and made the dedup filter append duplicates (#261). + """ + if not isinstance(group, dict): + return False + hooks = group.get("hooks") + return any( + isinstance(h, dict) and command_is_omind_hook(str(h.get("command", ""))) + for h in (hooks if isinstance(hooks, list) else []) + ) + + def gemini_config_dir() -> Path: """Gemini CLI's config directory: ``$GEMINI_HOME`` or ``~/.gemini``.""" base = os.environ.get("GEMINI_HOME") @@ -470,7 +487,7 @@ def install_priming(self) -> None: if not ( isinstance(e, dict) and isinstance(e.get("command"), str) - and HOOK_MARKER in e["command"] + and command_is_omind_hook(e["command"]) ) ] merged = kept + [desired] @@ -1096,7 +1113,7 @@ def install_priming(self) -> None: kept = [ g for g in existing - if not (isinstance(g, dict) and HOOK_MARKER in json.dumps(g)) + if not _group_has_omind_hook(g) ] merged = kept + [desired] if merged == existing and not self.config.force: @@ -1127,7 +1144,7 @@ def install_accounting(self) -> None: kept = [ group for group in existing - if not (isinstance(group, dict) and HOOK_MARKER in json.dumps(group)) + if not _group_has_omind_hook(group) ] merged = kept + [desired] if merged == existing and not self.config.force: @@ -1208,10 +1225,7 @@ def _priming_wired(self) -> bool: _root, hooks_cfg = self._read_hooks_file() except ProvisionError: return False - return any( - isinstance(g, dict) and HOOK_MARKER in json.dumps(g) - for g in (hooks_cfg.get("SessionStart") or []) - ) + return any(_group_has_omind_hook(g) for g in (hooks_cfg.get("SessionStart") or [])) def _accounting_wired(self) -> bool: try: @@ -1219,8 +1233,7 @@ def _accounting_wired(self) -> bool: except ProvisionError: return False return any( - isinstance(group, dict) and HOOK_MARKER in json.dumps(group) - for group in (hooks_cfg.get("PostToolUse") or []) + _group_has_omind_hook(group) for group in (hooks_cfg.get("PostToolUse") or []) ) # -- persisted hook trust ---------------------------------------------- diff --git a/src/omind/hooks.py b/src/omind/hooks.py index f71b414..466d726 100644 --- a/src/omind/hooks.py +++ b/src/omind/hooks.py @@ -46,6 +46,17 @@ _T = TypeVar("_T") HOOK_MARKER = "omind hook" # substring used by provision.py to find our entries +#: The omind executable followed by the ``hook`` subcommand. Windows resolves +#: the executable to ``omind.EXE`` / ``omind.cmd``, so the literal +#: :data:`HOOK_MARKER` substring never occurs there and a bare substring test +#: false-negatives — which made the Codex dedup filter append a duplicate hook +#: entry on every re-run (#261). +HOOK_COMMAND_RE = re.compile(r"omind(?:\.exe|\.cmd|\.bat)?[\"']?\s+hook\b", re.IGNORECASE) + + +def command_is_omind_hook(command: str) -> bool: + """True when ``command`` is an ``omind hook ...`` invocation omind owns.""" + return HOOK_MARKER in command or bool(HOOK_COMMAND_RE.search(command)) HANDLED_EVENTS = ("PostToolUse", "Stop", "SessionStart") #: Hermes Agent has no SessionStart hook; it fires ``pre_llm_call`` before every #: LLM turn and consumes a ``{"context": ...}`` payload on stdout. omind installs diff --git a/src/omind/provision.py b/src/omind/provision.py index 81cb926..b241dac 100644 --- a/src/omind/provision.py +++ b/src/omind/provision.py @@ -31,7 +31,7 @@ from typing import Any, ClassVar, TextIO from omind import __version__, guard, paths, policy, seeds -from omind.hooks import HANDLED_EVENTS, HOOK_MARKER, JOURNAL_DIRNAME +from omind.hooks import HANDLED_EVENTS, JOURNAL_DIRNAME, command_is_omind_hook from omind.hooks import failure_log_path as hook_failure_log_path from omind.journal import find_stray_journals, migrate_journals from omind.proc import run_command @@ -265,14 +265,10 @@ def claude_settings_path() -> Path: return Path.home() / ".claude" / "settings.json" -# A hook command we own: the omind executable (Windows resolves it to -# omind.EXE / omind.cmd, so the literal HOOK_MARKER substring isn't enough) -# followed by the `hook` subcommand. -_HOOK_COMMAND_RE = re.compile(r"omind(?:\.exe|\.cmd|\.bat)?[\"']?\s+hook\b", re.IGNORECASE) - - -def _command_is_omind_hook(command: str) -> bool: - return HOOK_MARKER in command or bool(_HOOK_COMMAND_RE.search(command)) +# A hook command we own: the omind executable followed by the `hook` +# subcommand. The Windows-tolerant predicate lives in hooks.py so the Codex +# provisioner (agents.py) shares it instead of re-deriving a substring test. +_command_is_omind_hook = command_is_omind_hook def _entry_has_omind_marker(entry: object) -> bool: @@ -446,12 +442,21 @@ class Provisioner: config: SetupConfig log: Logger = print actions: list[str] = field(default_factory=list) + #: soft tools found missing by :meth:`check_prereqs`; dependent steps skip. + missing_tools: set[str] = field(default_factory=set) #: tool -> why it is needed; subclasses for other agents override this. REQUIRED_TOOLS: ClassVar[dict[str, str]] = { "claude": "the Claude Code CLI registers the MCP server", "git": "the mesh replicates the memory folder over git", } + #: Tools whose absence degrades setup instead of aborting it: every step + #: that does not shell out to them still runs, the dependent steps skip + #: with a warning, and doctor flags the gap. Before this, --dry-run merely + #: WARNED about a missing claude while the real run refused outright and + #: did nothing at all — an asymmetry that ambushed anyone who validated + #: with --dry-run first (#258). + SOFT_TOOLS: ClassVar[frozenset[str]] = frozenset({"claude"}) DONE_MESSAGE: ClassVar[str] = ( "Done. Restart Claude Code to load the OMI memory tools. To replicate " "to other machines: `omind mesh add-peer`, then `omind mesh install-service`." @@ -509,11 +514,27 @@ def _write_managed(self, path: Path, content: str, *, mode: int | None = None) - # -- steps -------------------------------------------------------------- def check_prereqs(self) -> None: - """Raise (unless dry-run) when a required executable is missing.""" + """Raise (unless dry-run) when a required executable is missing. + + Tools in :data:`SOFT_TOOLS` never raise: their absence is recorded in + ``self.missing_tools``, the steps that shell out to them skip with a + warning, and everything else proceeds — the same on a real run as on + ``--dry-run`` (#258). + """ required = self.REQUIRED_TOOLS missing = [tool for tool in required if shutil.which(tool) is None] - if missing: - details = "; ".join(f"{t} ({required[t]})" for t in missing) + soft = [tool for tool in missing if tool in self.SOFT_TOOLS] + hard = [tool for tool in missing if tool not in self.SOFT_TOOLS] + if soft: + self.missing_tools.update(soft) + details = "; ".join(f"{t} ({required[t]})" for t in soft) + self.log( + f" WARNING: missing tool(s): {details}. Continuing — the " + "steps that need them will be skipped; install them and " + "re-run `omind setup` (doctor flags the gap meanwhile)." + ) + if hard: + details = "; ".join(f"{t} ({required[t]})" for t in hard) message = ( f"missing required tool(s): {details}. Install them, then re-run." ) @@ -521,7 +542,7 @@ def check_prereqs(self) -> None: self.log(f" WARNING: {message}") else: raise ProvisionError(message) - else: + if not missing: self.log(f" prerequisites present: {', '.join(required)}") def ensure_vault(self) -> None: @@ -640,6 +661,12 @@ def retire_legacy_server(self) -> None: self._run(["claude", "mcp", "remove", LEGACY_SERVER_NAME, "-s", "user"]) def register_mcp(self) -> None: + if "claude" in self.missing_tools: + self.log( + " SKIP: MCP registration shells out to the claude CLI, which " + "is not installed — install it and re-run `omind setup`." + ) + return existing = self.registered_server() if existing is not None and self._matches_desired(existing) and not self.config.force: self.log( @@ -1087,6 +1114,8 @@ def ensure_omi_guard_installed(self) -> None: def verify(self) -> None: if self.config.dry_run: return + if "claude" in self.missing_tools: + return # nothing was registered; check_prereqs already warned result = self._run( ["claude", "mcp", "get", self.config.server_name], check=False, diff --git a/tests/test_agents.py b/tests/test_agents.py index 244f2d3..13706a3 100644 --- a/tests/test_agents.py +++ b/tests/test_agents.py @@ -968,3 +968,61 @@ def test_agent_provisioners_never_call_claude( def test_hermes_provisioner_done_message_names_hermes(tmp_path: Path) -> None: assert "Hermes" in HermesProvisioner.DONE_MESSAGE assert "OpenClaw" in OpenClawProvisioner.DONE_MESSAGE + + +def test_codex_recognizes_windows_launcher_hook_entries(tmp_path: Path) -> None: + """Windows resolves the executable to omind.EXE, so the command text never + contains the literal "omind hook" marker. The provisioner must still + recognize its own SessionStart/PostToolUse entries there — the old + substring-on-json.dumps idiom failed verification AND appended a duplicate + entry on every re-run (#261).""" + agents.codex_config_dir().mkdir(parents=True, exist_ok=True) + hooks_path = agents.codex_hooks_path() + win_session = ( + 'C:\\Users\\ci\\.local\\bin\\omind.EXE hook SessionStart ' + '--vault "C:\\Users\\ci\\FreshVault" --folder "OMI"' + ) + win_accounting = ( + 'C:\\Users\\ci\\.local\\bin\\omind.EXE hook PostToolUse ' + '--vault "C:\\Users\\ci\\FreshVault" --folder "OMI"' + ) + hooks_path.write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + {"hooks": [{"type": "command", "command": win_session, "timeout": 15}]} + ], + "PostToolUse": [ + {"hooks": [{"type": "command", "command": win_accounting, "timeout": 15}]} + ], + } + } + ), + encoding="utf-8", + ) + + config = _config(tmp_path, "codex") + provisioner = agents.CodexProvisioner(config, log=_quiet) + assert provisioner._priming_wired() + assert provisioner._accounting_wired() + + # A re-run replaces the Windows-form entries instead of appending duplicates. + run_setup_for(config, log=_quiet) + data = json.loads(hooks_path.read_text(encoding="utf-8")) + hooks = data["hooks"] + assert len(hooks["SessionStart"]) == 1 + assert len(hooks["PostToolUse"]) == 1 + + +def test_command_is_omind_hook_forms() -> None: + from omind.hooks import command_is_omind_hook + + posix = '/home/akclark/.local/bin/omind hook SessionStart --vault "/v" --folder "OMI"' + win = 'C:\\Users\\ci\\.local\\bin\\omind.EXE hook PostToolUse --vault "C:\\V" --folder "OMI"' + quoted = "'C:\\Users\\ci\\.local\\bin\\omind.exe' hook SessionStart" + assert command_is_omind_hook(posix) + assert command_is_omind_hook(win) + assert command_is_omind_hook(quoted) + assert not command_is_omind_hook("some-other-tool hook SessionStart") + assert not command_is_omind_hook("omind guard adapter --harness codex") diff --git a/tests/test_cli_integration.py b/tests/test_cli_integration.py index aac1d0d..dc11b0c 100644 --- a/tests/test_cli_integration.py +++ b/tests/test_cli_integration.py @@ -289,6 +289,29 @@ def test_setup_missing_tools_exits_1_with_error( assert "missing required tool" in capsys.readouterr().err +def test_setup_missing_claude_degrades_instead_of_refusing( + isolate_config: Path, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """claude is a SOFT tool: without it, setup still seeds the vault and + installs hooks, skipping only MCP registration — matching what --dry-run + leads the operator to expect (#258).""" + real_which = provision.shutil.which + monkeypatch.setattr( + provision.shutil, + "which", + lambda name: None if name == "claude" else real_which(name), + ) + rc = main(["setup", "--vault", str(tmp_path / "vault"), "--no-mesh"]) + out = capsys.readouterr().out + assert rc == 0 + assert "SKIP: MCP registration" in out + assert "WARNING: missing tool(s): claude" in out + assert (tmp_path / "vault" / "OMI" / "index.md").exists() + + # -- quickstart --------------------------------------------------------------- diff --git a/uv.lock b/uv.lock index 5112707..0b25064 100644 --- a/uv.lock +++ b/uv.lock @@ -1054,7 +1054,7 @@ name = "exceptiongroup" version = "1.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "typing-extensions", marker = "python_full_version < '3.11'" }, + { name = "typing-extensions" }, ] sdist = { url = "https://files.pythonhosted.org/packages/50/79/66800aadf48771f6b62f7eb014e352e5d06856655206165d775e675a02c9/exceptiongroup-1.3.1.tar.gz", hash = "sha256:8b412432c6055b0b7d14c310000ae93352ed6754f70fa8f7c34141f91c4e3219", size = 30371, upload-time = "2025-11-21T23:01:54.787Z" } wheels = [ @@ -2354,7 +2354,7 @@ wheels = [ [[package]] name = "omind" -version = "8.7.0" +version = "8.7.1" source = { editable = "." } dependencies = [ { name = "cryptography" },