diff --git a/docs/install-verification.md b/docs/install-verification.md new file mode 100644 index 0000000..517fd81 --- /dev/null +++ b/docs/install-verification.md @@ -0,0 +1,176 @@ +# omind install verification matrix + +Agent-executable acceptance tests for a **new omind install** (or a periodic +drift audit of an existing one). Every row is a command an agent can run +unattended plus a machine-checkable pass criterion. + +`omind doctor` is the fast smoke test — it covers roughly group A–C by +inspection. This matrix exists because doctor checks *presence*, not +*behavior*: a hook can be wired and still not block, an MCP server can be +registered and still not answer. The behavioral groups (D, E, F) are the ones +that catch real drift. + +## Safety rules for the executing agent + +1. **Never run destructive commands to test the guard.** Use `omind guard + explain --command ''` (pure dry-run, nothing executes) and `omind guard + selftest` (canned events). No row in this matrix may execute `sudo`, `gh repo + delete`, or a raw API DELETE. +2. **Write tests go to a throwaway vault**, never the real OMI folder: + ```sh + export TESTVAULT="$(mktemp -d)/vault"; mkdir -p "$TESTVAULT/OMI" + ``` + Pass `--vault "$TESTVAULT" --folder OMI` on every write/index/mesh row. + Rows marked **RO** are read-only and may run against the live vault. +3. **Do not `omind guard pause`** as part of testing, and do not leave the guard + paused. If `guard status` reports PAUSED, that is a finding (see D0), not a + precondition to satisfy. +4. Record every row as PASS / FAIL / SKIP with the observed output. A SKIP needs + a reason (optional dependency absent, no mesh peers configured, etc.). + +--- + +## A. Binary and environment + +| ID | Command | Pass criterion | +|----|---------|----------------| +| A1 | `command -v omind` | Resolves; path is on `PATH` for non-interactive shells too (`zsh -lc 'command -v omind'`) | +| A2 | `omind --version` | Prints a version; matches the intended release. A SessionStart banner advertising a newer version than `--version` reports is drift | +| A3 | `omind help` | Exits 0 and lists all subcommands: setup, quickstart, node, mesh, serve, doctor, backup, export, import, reindex, convert, note, consolidate, search, bench, lint, graph, recover, checkpoint, ai, rollup, hook, loop, guard | +| A4 | `command -v claude git jq` | All three present — `claude` backs the verifier model, `jq` is the guard fast path, `git` backs mesh/merge | +| A5 | `omind help ` for each subcommand in A3 | Every subcommand has authoritative help and exits 0 (no `NotImplementedError`, no empty body) | + +## B. Wiring — what `omind setup` is supposed to have installed + +| ID | Command | Pass criterion | +|----|---------|----------------| +| B1 | `omind doctor` | Exit 0, **zero problems**. Warnings triaged individually against this table | +| B2 | `omind doctor` line "MCP server 'omi'" | `[✓]`, not "differs from the expected `omind node` command". This is the single most common drift after a manual edit of `~/.claude.json` | +| B3 | `jq '.mcpServers.omi' ~/.claude.json` | Command array is `[, node, --vault, , --folder, OMI]` — an absolute interpreter path pointing into a venv that no longer exists is the classic post-reinstall break | +| B4 | `jq '.hooks \| keys' ~/.claude/settings.json` | Contains `PreToolUse`, `PostToolUse`, `Stop`, `SessionStart`, `UserPromptSubmit` | +| B5 | `jq -r '.hooks[][]\|.hooks[]?.command' ~/.claude/settings.json` | Contains `omind hook PostToolUse`, `omind hook Stop`, `omind hook SessionStart`, plus the four managed scripts: `omi-guard.sh`, `omi-gate-reset.sh`, `omi-enforce.py`, `secret-output-guard.sh` (and `git-fresh-base.sh` where repo rules apply) | +| B6 | `ls -l ~/.claude/hooks/` | All hook scripts present and executable (`0755`). Managed guard scripts are expected to be root-owned/write-protected — a user-writable `omi-guard.sh` defeats self-protection | +| B7 | `cat ~/.claude/hooks/.omind-provision.json` | Manifest present; its recorded SHAs match the shipped hook resources. Mismatch = **hookset drift** (someone hand-edited a managed script); repair with `omind setup` | +| B7a | `omind doctor` line "auto-memory hooks run a non-canonical omind" | Absent. Present = hooks are pinned to a different install than `~/.local/bin/omind`, so `self-update` will never reach them — the failure that let one box run 8.1.1 hooks under an 8.2.0 binary indefinitely | +| B7b | Confirm `omind setup` can actually write `~/.claude/settings.json` | Writable by the user. If it is root-owned **and** immutable (`chattr +i`), setup dies with `PermissionError` and every "run `omind setup`" repair instruction in this matrix is impossible until an operator clears it — verify before trusting any B/C row's remediation | +| B8 | `test -f ~/.claude/skills/omind/SKILL.md` | Present — this is how agents discover authoritative command syntax | +| B9 | `omind doctor` line "seed files present" | `[✓]` — `index.md` + note template exist in the OMI folder | +| B10 | `test -f "$OMI/.obsidian/app.json"` | Obsidian config seeded | +| B11 | `git -C "$OMI" config --get-regexp 'merge\.(omi\|ours)'` + `cat "$OMI/.gitattributes"` | Both merge drivers configured and `.gitattributes` routes `*.md` to the `omi` driver — without this, mesh sync silently produces conflict markers in notes | + +## C. Hook behavior (not just presence) + +Drive each hook the way the harness does: JSON on stdin. + +| ID | Command | Pass criterion | +|----|---------|----------------| +| C1 | `echo '{"hook_event_name":"SessionStart","session_id":"verify"}' \| omind hook SessionStart --vault "$TESTVAULT" --folder OMI` | Exit 0; emits the priming capsule (index/playbook/memory-workflow sections). Empty output on a seeded vault = broken priming | +| C2 | `echo '{"hook_event_name":"PostToolUse","session_id":"verify","tool_name":"Bash","tool_input":{"command":"ls"}}' \| omind hook PostToolUse --vault "$TESTVAULT" --folder OMI` | Exit 0; a journal entry appears for session `verify` | +| C3 | `echo '{"hook_event_name":"Stop","session_id":"verify"}' \| omind hook Stop --vault "$TESTVAULT" --folder OMI` | Exit 0; session journal is finalized | +| C4 | `~/.claude/hooks/omi-gate-reset.sh < /dev/null` | Exit 0; the per-turn consult gate is reset (verify with `omind guard status` before/after) | +| C5 | `omind doctor` line "no recorded hook failures" | `[✓]`. A recorded failure means a hook crashed in a real session — read it, don't clear it | +| C6 | Latency: `time` each of C1–C3 | Each well under its configured timeout (10–20s in settings). A hook at 80%+ of timeout will flake under load | +| C7 | `omind loop status` | Reports DISARMED on a fresh install. ARMED on a new install means the Stop hook will refuse to stop — a hang, not a feature | + +## D. Guard policy engine — the block path + +**This is the group that matters.** A guard that is wired but not blocking is +worse than no guard, because it reads as protection. + +| ID | Command | Pass criterion | +|----|---------|----------------| +| D0 | `omind guard status` | **NOT PAUSED.** `guard pause` time-boxes off the consult-gate + verifier; a long pause left armed disables the soft path indefinitely. Also confirm "self-protection: guard config is write-protected" | +| D1 | `omind guard selftest` | **Every** harness row `[ok]`, and exit 0. Rows: claude/exit2, hermes/claude_json, opencode/json_signal, codex/codex_hook, gemini/gemini, openclaw/openclaw. Beware measuring `$?` through a pipe (`selftest \| head` reports *head's* status) — redirect instead | +| D2 | `omind guard policy` | Lists the 6 seed rules + any learned rules. Seed count < 6 = a broken policy load | +| D3 | `omind guard explain --command 'sudo ls'` | `DENY`, rule `sudo-use-fleet-sudo`, tier `sudo` | +| D4 | `omind guard explain --command 'pkexec id'` / `'doas id'` / `'run0 id'` / `'su -c id root'` | `DENY` on all four, rule `privesc-alternatives` | +| D5 | `omind guard explain --command 'gh repo delete a/b'` | `DENY`, rule `gh-repo-delete` | +| D6 | `omind guard explain --command 'gh api repos/o/r -X DELETE'` **and** `'gh api -X DELETE repos/o/r'` | `DENY` both orders, rule `gh-api-repo-delete` (order-independence is a regression-tested property) | +| D7 | `omind guard explain --command 'curl -X DELETE https://api.github.com/repos/o/r'` | `DENY`, rule `curl-api-repo-delete` | +| D8 | `omind guard explain --command 'gh auth setup-git'` | `DENY`, rule `gh-auth-setup-git` | +| D9 | **False-positive anchors.** `omind guard explain` on each of: `grep -rn sudo .`, `cat /var/log/sudo.log`, `git commit -m "fix sudo"`, `pass show sudo/akclark`, `fleet-sudo whoami`, `man su`, `tmux new -s run0`, `gh pr view` | `ALLOW` on **all**. These are command-position-anchoring regressions that make the guard unusable if they break | +| D10 | `OMI_SUDO_OK=1 omind guard explain --command 'sudo ls'` | Allowed — the documented opt-in escape hatch still works | +| D11 | `omind guard log --limit 20` | Recent decisions are being written; `deny` and `violation` events both appear over the install's history | +| D12 | `omind doctor` line "policy engine allowed an unconsulted action" | Absent. If present, the block path is broken — cross-check with D0/D1 before concluding it's a code fault | + +## E. MCP surface + +Start the server the way Claude Code does and exercise it. Use a throwaway +vault so write tools are safe. + +| ID | Check | Pass criterion | +|----|-------|----------------| +| E1 | `omind node --vault "$TESTVAULT" --folder OMI` over stdio; issue `tools/list` | Server handshakes and lists the full tool set: search-vault, recall-note, read-note, list-notes, list-tags, create-note, edit-note, delete-note, restore-note, backlinks, graph, graph-neighbors, help | +| E2 | From a live agent session: `mcp__omi__list-notes` | Returns notes — proves the *registered* server (B3) actually runs, which E1 alone does not | +| E3 | `mcp__omi__search-vault` for a known-present phrase **RO** | Returns the expected note | +| E4 | `mcp__omi__recall-note` **RO** | Returns a capsule, not an error | +| E5 | If `mcp-conformance` is available: run it against `conformance.toml` | All probes pass; output under `max_output_chars` (200000) | +| E6 | `jq '.permissions.allow' ~/.claude/settings.json` | Read-only omi tools pre-allowed (read-note, search-vault, list-notes, recall-note, help) so routine recall doesn't prompt. Write tools must **not** be blanket-allowed | + +## F. Write path, locking, and recovery + +All against `$TESTVAULT`. + +| ID | Command | Pass criterion | +|----|---------|----------------| +| F1 | `omind note --vault "$TESTVAULT" --folder OMI ...` create a note | Note written with valid YAML frontmatter incl. `type`; appears in `index.md` Recent Memories | +| F2 | Re-run F1 with an edit + a stale `expected_version` | Rejected as a concurrent-write conflict. Silent overwrite = data-loss bug | +| F3 | `test -f "$TESTVAULT/OMI/.omi.lock"` and run two `omind note` writes concurrently | Single-writer lock holds; both writes land, neither corrupts | +| F4 | `omind recover --vault "$TESTVAULT" --folder OMI` with no pending txn | Reports nothing to recover, exits 0 | +| F5 | `mcp__omi__delete-note` then `restore-note` on a scratch note | Delete is a soft archive (file still on disk); restore brings it back | +| F6 | `omind convert --vault "$TESTVAULT" --folder OMI` run twice | Idempotent — second run reports no changes | +| F7 | `omind lint --vault "$TESTVAULT" --folder OMI` | Exits cleanly; on the real vault **RO**, review broken wikilinks / orphans / dupes as findings rather than failures | + +## G. Retrieval + +| ID | Command | Pass criterion | +|----|---------|----------------| +| G1 | `omind doctor` line "search index: FTS5 available" | `[✓]`. FTS5 missing = the keyword path is degraded to a scan | +| G2 | `omind doctor` line "semantic search" | `[✓]` if `omind[embed]` was intended. "off (keyword path) — model2vec not importable" is a legitimate SKIP only if the install deliberately omitted the extra; it costs ~20pp recall@1 | +| G3 | `omind doctor` line "search index: … stale note(s)" | Zero stale notes and index age consistent with the write timer. Non-zero stale = run `omind reindex --rebuild` and re-check | +| G4 | `omind search ''` **RO** | Returns the expected note in the top hits | +| G5 | `omind reindex --vault "$TESTVAULT" --folder OMI` | Exits 0; index rebuilt under the write lock | +| G6 | `omind bench --vault "$TESTVAULT" --folder OMI` | Reports index-build, search latency, capsule build, recall token cost. Record as the **install's baseline** — this row's value is the number, compared over time | +| G7 | `omind graph stats` / `graph orphans` / `graph dangling` **RO** | All exit 0 and return structured output | + +## H. Mesh, backup, transfer + +Skip with a reason if the install is standalone (no peers). + +| ID | Command | Pass criterion | +|----|---------|----------------| +| H1 | `omind mesh status` | Node identity present; each peer reports ahead/behind counts with a recent fetch | +| H2 | `omind doctor` line "last sync" | Recent relative to the sync interval. Hours-old sync on an active mesh = a broken timer | +| H3 | `omind doctor` line "no unresolved conflict markers" | `[✓]`. Any `<<<<<<<` in a note means the omi merge driver (B11) isn't doing its job | +| H4 | `omind mesh sync` (dry-run first if supported) | Completes without conflict | +| H5 | `omind backup` status / `omind doctor` "last backup succeeded" | Succeeded recently, to the intended destination. A backup that has never run on a new install is expected — verify the *schedule* exists | +| H6 | `omind export --vault "$TESTVAULT" --folder OMI` → `omind import` into a second temp vault | Round-trips: note count and content match the source | + +## I. Scheduled / ancillary features + +| ID | Command | Pass criterion | +|----|---------|----------------| +| I1 | `omind checkpoint --vault "$TESTVAULT" --folder OMI` | Produces/updates a daily worklog note | +| I2 | `systemctl --user list-timers \| grep -i omind` (or the platform equivalent) | The checkpoint timer is installed and scheduled if `install-timer` was intended | +| I3 | `omind rollup --vault "$TESTVAULT" --folder OMI` | Compacts dailies; default archives rather than deletes | +| I4 | `omind consolidate --vault "$TESTVAULT" --folder OMI` | Proposes merges **without mutating** the vault (verify with `git -C status`/checksums before and after) | +| I5 | `omind ai` | Reports token usage and the active model-expense profile | +| I6 | `omind serve` on the throwaway vault | Binds **localhost only**, unauthenticated by design. A non-loopback bind is a security finding, not a config preference | + +--- + +## Reporting format + +``` +omind install verification — +A: 5/5 B: 11/11 C: 6/7 (C7 FAIL) D: 11/13 (D0,D1 FAIL) E: 6/6 +F: 7/7 G: 6/7 (G2 SKIP) H: 6/6 I: 5/6 (I2 SKIP — no timer) + +FAIL D0 — guard PAUSED for 185h; consult-gate + verifier disabled +FAIL D1 — selftest [FAIL] claude/exit2, gemini/gemini (downstream of D0) +SKIP G2 — omind[embed] not installed; keyword path only +``` + +Group D failures are release-blocking. Groups B/C failures are usually repaired +by re-running `omind setup`; re-run the failed rows afterward rather than +assuming the repair took. diff --git a/src/omind/agents.py b/src/omind/agents.py index f4f85ce..68c044b 100644 --- a/src/omind/agents.py +++ b/src/omind/agents.py @@ -25,7 +25,6 @@ import json import os import shlex -import shutil import sys from pathlib import Path from typing import Any, ClassVar @@ -45,6 +44,7 @@ SetupConfig, _diagnose_omi_folder, _diagnose_tools, + canonical_omind_exe, diagnose, ) @@ -339,7 +339,7 @@ def _omind_hook_command(self, event: str) -> str: Both folder values are quoted so a path like ``My Vault`` cannot word-split into a stray positional. """ - omind_exe = shutil.which("omind") or "omind" + omind_exe = canonical_omind_exe() return ( f'{omind_exe} hook {event} --vault "{self.config.vault}" ' f'--folder "{self.config.folder}"' @@ -506,7 +506,7 @@ def _write_guard_script(self) -> None: except Exception as exc: self.log(f" WARNING: could not read omi-guard-hermes.sh from package data: {exc}") return - omind_exe = shutil.which("omind") or "omind" + omind_exe = canonical_omind_exe() content = content.replace("__OMIND_BIN__", omind_exe).replace( "__OMI_DIR__", str(self.config.omi_dir) ) @@ -735,7 +735,7 @@ def install_guard(self) -> None: """ path = openclaw_config_path() data = self._read_settings(path) - command = f"{shutil.which('omind') or 'omind'} guard adapter --harness openclaw" + command = f"{canonical_omind_exe()} guard adapter --harness openclaw" desired = {"event": "pre_tool", "command": command, "enabled": True} hooks = data.get("hooks") if not isinstance(hooks, dict): @@ -805,7 +805,7 @@ def integrate(self) -> None: def _guard_hook_group(self) -> dict[str, Any]: """One ``BeforeTool`` matcher group running the omind gemini adapter on every tool. Gemini pipes the event JSON on stdin; the adapter reads it.""" - omind = shutil.which("omind") or "omind" + omind = canonical_omind_exe() return { "matcher": ".*", "hooks": [ @@ -899,7 +899,7 @@ def registered_server(self) -> dict[str, Any] | None: def desired_server_entry(self) -> dict[str, Any]: # OpenCode local MCP server: a `type: local` + command array. - omind = shutil.which("omind") or "omind" + omind = canonical_omind_exe() return { "type": "local", "command": [ @@ -952,7 +952,7 @@ def install_guard(self) -> None: except Exception as exc: self.log(f" WARNING: could not read omi-guard.opencode.js from package data: {exc}") return - omind_exe = shutil.which("omind") or "omind" + omind_exe = canonical_omind_exe() content = content.replace("__OMIND_BIN__", omind_exe).replace( "__OMI_DIR__", str(self.config.omi_dir) ) @@ -1010,7 +1010,7 @@ def integrate(self) -> None: def _guard_hook_group(self) -> dict[str, Any]: """One Claude-schema matcher group running the omind codex adapter on all tools. Codex pipes the event JSON on stdin; the adapter reads it directly.""" - omind = shutil.which("omind") or "omind" + omind = canonical_omind_exe() return { "hooks": [ { @@ -1412,7 +1412,7 @@ def _read_toml_config(self) -> tomlkit.TOMLDocument: ) from exc def desired_mcp_entry(self) -> dict[str, Any]: - omind = shutil.which("omind") or "omind" + omind = canonical_omind_exe() return { "command": omind, "args": ["node", "--vault", str(self.config.vault), "--folder", self.config.folder], @@ -1492,7 +1492,7 @@ def registered_server(self) -> dict[str, Any] | None: return server if isinstance(server, dict) else None def desired_server_entry(self) -> dict[str, Any]: - omind = shutil.which("omind") or "omind" + omind = canonical_omind_exe() entry: dict[str, Any] = {} if self.STDIO_TYPE: entry["type"] = "stdio" diff --git a/src/omind/backup.py b/src/omind/backup.py index 13645ec..3844b10 100644 --- a/src/omind/backup.py +++ b/src/omind/backup.py @@ -41,7 +41,7 @@ from omind.notes import upsert_note from omind.paths import INDEX_FILENAME from omind.proc import DEFAULT_TIMEOUT, run_command -from omind.provision import CheckResult, Logger, SetupConfig +from omind.provision import CheckResult, Logger, SetupConfig, canonical_omind_exe from omind.store import NoteError, NoteFields, NoteNotFoundError, OmiStore PASS_FILENAME = "backup.pass" @@ -410,7 +410,7 @@ def install_timer(config: SetupConfig, log: Logger = print) -> None: _require_config() # don't install a timer that can only ever fail unit_dir = systemd_user_dir() unit_dir.mkdir(parents=True, exist_ok=True) - omind_exe = shutil.which("omind") or "omind" + omind_exe = canonical_omind_exe() service = ( "[Unit]\n" "Description=omind encrypted vault backup\n" diff --git a/src/omind/guard.py b/src/omind/guard.py index faba775..aa7c2ae 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -467,6 +467,13 @@ def reset_offtopic(session: str) -> None: #: self-heals within the hour. _DEFAULT_PAUSE_SECONDS = 1800 +#: Hard ceiling on a single `omind guard pause --for`. A pause is meant to be a +#: work-burst window; past a few hours it is indistinguishable from disabling the +#: gate, and it silently masks doctor's enforcement check for the duration. One +#: box was found paused for 185h, which is how that failure mode was discovered. +#: Re-pausing is always allowed — the cap forces the operator to mean it. +_MAX_PAUSE_SECONDS = 4 * 3600 + def _pause_path() -> Path: """The OPERATOR pause sentinel. While it exists and is unexpired, the consult @@ -1552,6 +1559,10 @@ def _fmt_secs(secs: int) -> str: return f"{secs}s" +#: Public alias: the SessionStart priming banner formats the remaining pause. +fmt_secs = _fmt_secs + + def _run_pause(duration: str) -> int: """``omind guard pause [--for 30m]``: skip the consult-gate + verifier for a time-boxed fast window (mission-critical speed / token savings). The HARD @@ -1564,6 +1575,13 @@ def _run_pause(duration: str) -> int: resume_gate() sys.stdout.write("consult-gate re-armed (pause duration was 0).\n") return 0 + capped = min(seconds, _MAX_PAUSE_SECONDS) + if capped != seconds: + sys.stdout.write( + f"guard pause: {_fmt_secs(seconds)} exceeds the {_fmt_secs(_MAX_PAUSE_SECONDS)} " + f"cap — pausing for {_fmt_secs(capped)} instead.\n" + ) + seconds = capped pause_gate(seconds) compliance.log_event( compliance.KIND_DECISION, diff --git a/src/omind/hooks.py b/src/omind/hooks.py index d75193b..7f7a556 100644 --- a/src/omind/hooks.py +++ b/src/omind/hooks.py @@ -374,6 +374,28 @@ def _update_nudge_line() -> str | None: return f"⚠️ {nudge}" if nudge else None +def _paused_gate_line() -> str | None: + """A loud banner while the consult-gate is paused, or ``None``. + + A pause is easy to engage and easy to forget: its only visible trace was + `omind guard status`, which nobody runs unprompted, so a box sat paused for + 185h with the enforcement check reading as broken rather than disabled. + Surfacing it in every session's priming makes the degraded state impossible + to miss. Defensive: never break priming over a status read.""" + try: + from omind import guard + + remaining = guard.pause_remaining() + except Exception: + return None + if remaining <= 0: + return None + return ( + f"⚠️ OMI consult-gate + verifier are PAUSED for another {guard.fmt_secs(remaining)} " + "(hard destructive blocks stay ON). Run `omind guard resume` to re-arm." + ) + + def _clip(text: str, limit: int) -> str: clean = text.strip() if len(clean) <= limit: @@ -550,7 +572,8 @@ def build_session_start_context( # node startup) so the update prompt is reliably visible. Defensive: a # version check must never break priming. nudge = _update_nudge_line() - prefix = f"{nudge}\n\n" if nudge else "" + banners = [line for line in (_paused_gate_line(), nudge) if line] + prefix = "\n\n".join(banners) + "\n\n" if banners else "" header = ( "OMI is the durable-memory source of truth. This is a compact session " diff --git a/src/omind/mesh.py b/src/omind/mesh.py index 1412812..d7c5834 100644 --- a/src/omind/mesh.py +++ b/src/omind/mesh.py @@ -20,7 +20,6 @@ import re import secrets import shlex -import shutil import socket import subprocess import sys @@ -886,7 +885,10 @@ def install_service(vault: Path, folder: str, log: Logger = print) -> None: omi_dir = (vault / folder).expanduser() if load_node_config(omi_dir) is None: raise MeshError(f"not a mesh node yet — run `omind mesh init` first ({omi_dir})") - omind_exe = shutil.which("omind") or "omind" + # Imported lazily: provision imports mesh, so a module-level import cycles. + from omind.provision import canonical_omind_exe + + omind_exe = canonical_omind_exe() # Quoted like the hook command: systemd ExecStart and schtasks both # word-split an unquoted folder name containing a space. daemon_cmd = f'{omind_exe} mesh daemon --vault "{vault}" --folder "{folder}"' diff --git a/src/omind/provision.py b/src/omind/provision.py index 9b15bc5..0a55373 100644 --- a/src/omind/provision.py +++ b/src/omind/provision.py @@ -39,6 +39,35 @@ #: Identifies enforce-hook commands inside hook entries. ENFORCE_HOOK_MARKER = "omi-enforce.py" +#: Deny-rate threshold (percent of all logged actions) above which `doctor` calls +#: the consult-gate over-firing. Precision, not volume, decides whether the guard +#: is helping — see the compliance_log check. +_DENY_RATE_WARN_PCT = 25 + +#: The stable user-install path for the omind executable. On a `uv tool install` +#: box this is a symlink into the tool env that uv retargets on every upgrade, +#: which is exactly the indirection hooks want: absolute (so it fires in a shell +#: without ~/.local/bin on PATH) *and* stable (so an update never strands it). +CANONICAL_OMIND_EXE = Path.home() / ".local" / "bin" / "omind" + + +def canonical_omind_exe() -> str: + """The omind path to bake into hook commands and the MCP server entry. + + Deliberately NOT ``shutil.which("omind")``. which() returns whichever install + happens to sit first on PATH when `omind setup` runs, and that path is then + frozen into settings.json forever. On a dev box that captured an editable + checkout's venv binary, so `omind self-update` upgraded ~/.local/bin while + every hook kept executing a months-old build — silently, because the wiring + still "looked" correct. + + Falls back to which() only when the canonical path is absent (system or pipx + installs, which genuinely have no stable path to pin; `doctor` warns there). + """ + if CANONICAL_OMIND_EXE.exists(): + return str(CANONICAL_OMIND_EXE) + return shutil.which("omind") or "omind" + def _enforce_hook_dest() -> Path: """Where omind writes the enforcement hook script on this machine.""" @@ -275,6 +304,32 @@ def _entry_command_text(entry: object) -> str: return " ".join(parts) +#: The executable token immediately preceding ``hook `` in an installed +#: hook command — the omind binary that entry actually runs. +_HOOK_EXE_RE = re.compile(r"(?P\S+)\s+hook\s+\S") + + +def _hook_exe_path(command_text: str) -> str | None: + """The omind executable an installed hook command actually runs, if absolute. + + Hook commands look like `` hook --vault "…" --folder "…"``; the + token before ``hook`` is the binary. A bare ``omind`` (no directory) resolves + through PATH at run time and so cannot go stale — only absolute paths pin a + specific install, so those are all we report on. + + Both separators are checked, not just ``os.sep``: settings.json is portable + data, and a POSIX-style pin read on Windows (or the reverse) is precisely the + stale-install case this exists to catch. Matched with a regex rather than + ``shlex`` for the same reason — POSIX-mode ``shlex`` treats the backslashes in + ``C:\\venv\\Scripts\\omind`` as escapes and silently flattens the path away. + """ + for match in _HOOK_EXE_RE.finditer(command_text): + token = match.group("exe").strip("\"'") + if "/" in token or "\\" in token: + return token + return None + + #: The retired 1.x server registration (obsidian-mcp); setup removes it. LEGACY_SERVER_NAME = "obsidian" @@ -484,10 +539,11 @@ def registered_server(self) -> dict[str, object] | None: def _server_command(self) -> list[str]: """The `omind node` invocation the agent runs as its MCP server. - Absolute omind path when resolvable: the agent's spawn environment may - lack ~/.local/bin on PATH. + Canonical omind path (see :func:`canonical_omind_exe`): the agent's spawn + environment may lack ~/.local/bin on PATH, and the entry must survive an + update without re-registration. """ - omind_exe = shutil.which("omind") or "omind" + omind_exe = canonical_omind_exe() return [ omind_exe, "node", @@ -551,11 +607,11 @@ def register_mcp(self) -> None: def _hook_command(self, event: str) -> str: """The shell command Claude Code runs for one hook event. - Uses the absolute path to the ``omind`` executable when resolvable, so - the hook fires even if the shell Claude Code spawns lacks ``~/.local/bin`` - on PATH. Falls back to bare ``omind`` (still contains ``HOOK_MARKER``). + Uses :func:`canonical_omind_exe` so the hook fires even if the shell + Claude Code spawns lacks ``~/.local/bin`` on PATH, *and* keeps firing + after an update. """ - omind_exe = shutil.which("omind") or "omind" + omind_exe = canonical_omind_exe() # The " hook" prefix always contains HOOK_MARKER ("omind hook"), # which provision uses to find/replace omind's own entries. # Both values are quoted: the hook string goes through a shell, and an @@ -836,7 +892,7 @@ def _write_omi_guard_scripts(self) -> None: retires the legacy prototype adapter and stamps the provision manifest so upgrades can detect hook-set drift (#86/#87).""" self._remove_legacy_omi_guard() - omind_exe = shutil.which("omind") or "omind" + omind_exe = canonical_omind_exe() omi_dir = str(self.config.omi_dir) for resource, dest in ( ("omi-guard.sh", _omi_guard_dest()), @@ -1237,8 +1293,10 @@ def _diagnose_hooks(settings_path: Path, config: SetupConfig) -> CheckResult: ) expected_vault = str(config.vault) + canonical = canonical_omind_exe() missing: list[str] = [] path_mismatch = False + stale_exes: set[str] = set() for event in HANDLED_EVENTS: entries = hooks_cfg.get(event) found = None @@ -1246,8 +1304,16 @@ def _diagnose_hooks(settings_path: Path, config: SetupConfig) -> CheckResult: found = next((e for e in entries if _entry_has_omind_marker(e)), None) if found is None: missing.append(event) - elif expected_vault not in _entry_command_text(found): + continue + command_text = _entry_command_text(found) + if expected_vault not in command_text: path_mismatch = True + baked = _hook_exe_path(command_text) + # A hook pinned to some *other* omind install keeps running that build + # forever: self-update moves the canonical path, never this one. The + # wiring still looks correct, so only an explicit comparison catches it. + if baked and baked != canonical: + stale_exes.add(baked) if missing: return CheckResult( @@ -1262,6 +1328,14 @@ def _diagnose_hooks(settings_path: Path, config: SetupConfig) -> CheckResult: f"auto-memory hooks point at a different vault than {expected_vault!r}; " "run `omind setup`", ) + if stale_exes: + return CheckResult( + "hooks", + "fail", + "auto-memory hooks run a non-canonical omind " + f"({', '.join(sorted(stale_exes))}, not {canonical}) — self-update will " + "never reach them; run `omind setup`", + ) # Check the enforcement hook is present and the script exists on disk. enforce_dest = _enforce_hook_dest() post_entries = hooks_cfg.get("PostToolUse") @@ -1391,14 +1465,24 @@ def _diagnose_enforcement() -> list[CheckResult]: summary = compliance.summary() if summary["total"]: top = ", ".join(f"{rid}×{n}" for rid, n in summary["top_rules"][:3]) or "none" - results.append( - CheckResult( - "compliance_log", - "ok", - f"compliance log: {summary['total']} event(s), {summary['denies']} deny, " - f"{summary['violations']} violation(s); top: {top}", - ) + # The deny *rate*, not just the count. A gate that stops a large share of + # all actions stops carrying signal — agents learn to treat blocks as a + # toll booth to route around rather than a warning to read — and the raw + # totals hid that: 1107 denies looks like diligence until you notice it + # is one action in two. + rate = 100.0 * summary["denies"] / summary["total"] + status = "warn" if rate >= _DENY_RATE_WARN_PCT else "ok" + detail = ( + f"compliance log: {summary['total']} event(s), {summary['denies']} deny " + f"({rate:.0f}%), {summary['violations']} violation(s); top: {top}" ) + if status == "warn": + detail += ( + f" — deny rate over {_DENY_RATE_WARN_PCT}%: the gate is likely " + "over-firing; review with `omind guard log` and tune via " + "`omind guard suggest`" + ) + results.append(CheckResult("compliance_log", status, detail)) else: results.append( CheckResult("compliance_log", "ok", "compliance log: no violations recorded yet") diff --git a/src/omind/store.py b/src/omind/store.py index bd71ba7..79f8b3f 100644 --- a/src/omind/store.py +++ b/src/omind/store.py @@ -931,7 +931,41 @@ def safe_name(self, name: str) -> Path: Raises :class:`NoteError` on anything that looks like traversal: path separators, `..` segments, empty names, or a resolved path whose parent is not the OMI dir. + + Callers routinely pass a note *title* rather than its filename — the + guard's block message names the note to recall, ``[[wikilinks]]`` carry + titles, and search results show titles. A title may legally contain + characters (``/``, ``:``, ``?``, …) that :meth:`filename_for_title` + strips when the file is written, so the literal title is not a valid + filename and used to hard-fail here. That broke the guard's own + remediation loop: it blocked an action, told the agent to recall + "NEVER offer to end/pause the session …", and the recall was rejected + for the ``/`` in "end/pause" — an instruction impossible to satisfy. + + So when the raw name is rejected, fall back to the sanitized title, and + accept it only if that note actually exists. Traversal stays impossible: + the sanitizer strips separators outright, and the fallback re-runs the + full validation below on its result. Creates are unaffected — a + nonexistent note still raises. """ + try: + return self._validated_name(name) + except NoteError: + fallback = self._name_from_title(name) + if fallback is not None: + return fallback + raise + + def _name_from_title(self, name: str) -> Path | None: + """The existing note whose filename this *title* sanitizes to, if any.""" + try: + candidate = self._validated_name(self.filename_for_title(name)) + except NoteError: + return None + return candidate if candidate.exists() else None + + def _validated_name(self, name: str) -> Path: + """Strict filename validation, with no title fallback (see safe_name).""" name = (name or "").strip() if not name or name in {".", ".."}: raise NoteError("empty or invalid note name") diff --git a/src/omind/update.py b/src/omind/update.py index 548ef7d..5dfdd62 100644 --- a/src/omind/update.py +++ b/src/omind/update.py @@ -210,6 +210,49 @@ def update_command(install: InstallInfo, version: str) -> list[str] | None: return None # editable -> git pull; unknown -> manual +#: Shared opt-out with the `omind node` startup self-heal — one switch for +#: "I manage my own hooks", not two. +_NO_AUTOHEAL_ENV = "OMIND_NO_AUTOHEAL" + + +def _post_update_heal(*, log: Callable[[str], object] = print) -> None: + """Re-provision the wiring and pay any index migration, after an update. + + Both steps are fail-open: a successful update must never be reported as a + failure because a follow-up chore didn't work. + """ + if os.environ.get(_NO_AUTOHEAL_ENV): + return + from omind.provision import Provisioner, SetupConfig, default_vault_path + + vault = default_vault_path() + # Hook scripts, the MCP entry, and the skill are all rewritten by the new + # binary — otherwise a release that changes any of them lands only on boxes + # where someone remembered to re-run `omind setup` by hand. + try: + actions = Provisioner(config=SetupConfig(vault=vault), log=lambda _m: None).run() + if actions: + log(f"re-provisioned wiring ({len(actions)} change(s)).") + except Exception as exc: # noqa: BLE001 — never fail a good update + log(f"warning: re-provision failed ({exc}); run `omind setup` by hand.") + # Not a rebuild: opening the index runs the existing SCHEMA_VERSION/model + # check, which wipes and repopulates only when the format actually changed. + # Doing it here pays that cost in the update the user is already waiting on, + # instead of surprising the next search with it. + try: + from omind import searchindex + + index = searchindex.shared(vault / "OMI") + done = index.refresh() if index is not None else None + if done is not None and (done.reindexed or done.removed): + log( + f"search index refreshed: {done.reindexed} note(s) reindexed, " + f"{done.removed} removed ({done.seconds:.1f}s)." + ) + except Exception: # noqa: BLE001 — an index chore must never break an update + pass + + def self_update( *, check_only: bool = False, force: bool = False, log: Callable[[str], object] = print ) -> int: @@ -253,7 +296,9 @@ def self_update( log(f"update failed to launch: {exc}") return 1 if result.returncode == 0: - log(f"updated to {status.latest}. Restart the MCP server / agent session to load it.") + log(f"updated to {status.latest}.") + _post_update_heal(log=log) + log("Restart the MCP server / agent session to load it.") return 0 log(f"update command exited {result.returncode}.") return result.returncode diff --git a/tests/test_agents.py b/tests/test_agents.py index e16bbca..244f2d3 100644 --- a/tests/test_agents.py +++ b/tests/test_agents.py @@ -27,9 +27,13 @@ @pytest.fixture(autouse=True) def fake_tools(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(provision.shutil, "which", lambda name: f"/usr/bin/{name}") - # agents._omind_hook_command resolves omind via agents.shutil — patch it too - # so the hook/bootstrap wiring is deterministic regardless of the test host. - monkeypatch.setattr(agents.shutil, "which", lambda name: f"/usr/bin/{name}") + # Every harness now resolves omind through provision.canonical_omind_exe(), + # which prefers ~/.local/bin/omind and only falls back to which(). Point the + # canonical path at a location that cannot exist so these tests keep + # exercising the which() fallback instead of the test host's real install. + monkeypatch.setattr( + provision, "CANONICAL_OMIND_EXE", Path("/nonexistent/.local/bin/omind") + ) @pytest.fixture(autouse=True) diff --git a/tests/test_guard.py b/tests/test_guard.py index 2cefbe1..52f9ed7 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -1380,3 +1380,19 @@ def test_would_you_without_an_authorizing_verb_still_blocks_side_effects() -> No ) assert not blocked.allow guard.clear_gate("wouldneg") +def test_guard_pause_is_capped(capsys: pytest.CaptureFixture[str]) -> None: + """A week-long pause is a disable with extra steps: it silently masks the + enforcement check for the duration. One box was found paused for 185h.""" + assert guard.run_guard("pause", duration="185h") == 0 + out = capsys.readouterr().out + assert "cap" in out + remaining = guard.pause_remaining() + assert 0 < remaining <= guard._MAX_PAUSE_SECONDS + guard.resume_gate() + + +def test_guard_pause_under_the_cap_is_untouched(capsys: pytest.CaptureFixture[str]) -> None: + assert guard.run_guard("pause", duration="30m") == 0 + assert 0 < guard.pause_remaining() <= 1800 + assert "cap" not in capsys.readouterr().out + guard.resume_gate() diff --git a/tests/test_provision.py b/tests/test_provision.py index 77bd945..421bb6d 100644 --- a/tests/test_provision.py +++ b/tests/test_provision.py @@ -1070,3 +1070,43 @@ def test_cross_session_inbound_dry_run_writes_nothing( prov = Provisioner(_config(tmp_path, dry_run=True), log=_quiet) prov.ensure_cross_session_inbound() assert not isolate_settings.exists() +def test_canonical_omind_exe_prefers_the_stable_user_path( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """which() returns whatever sits first on PATH at setup time and that path is + then frozen into settings.json forever — which is how a dev venv captured a + production hook-set and kept running a stale build across every self-update.""" + canonical = tmp_path / ".local" / "bin" / "omind" + canonical.parent.mkdir(parents=True) + canonical.write_text("#!/bin/sh\n", encoding="utf-8") + monkeypatch.setattr(provision, "CANONICAL_OMIND_EXE", canonical) + monkeypatch.setattr(provision.shutil, "which", lambda _n: "/some/venv/bin/omind") + assert provision.canonical_omind_exe() == str(canonical) + + +def test_canonical_omind_exe_falls_back_when_absent( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """System/pipx installs have no stable path to pin.""" + monkeypatch.setattr(provision, "CANONICAL_OMIND_EXE", tmp_path / "nope") + monkeypatch.setattr(provision.shutil, "which", lambda _n: "/usr/bin/omind") + assert provision.canonical_omind_exe() == "/usr/bin/omind" + + +def test_hook_exe_path_reads_only_absolute_pins() -> None: + """A bare `omind` resolves through PATH at run time and cannot go stale. + + Both separators count on every platform: settings.json is portable data, so a + POSIX-style pin must still be recognised when doctor runs on Windows. + """ + assert ( + provision._hook_exe_path('/venv/bin/omind hook Stop --vault "/v" --folder "OMI"') + == "/venv/bin/omind" + ) + assert ( + provision._hook_exe_path(r'C:\venv\Scripts\omind hook Stop --vault "C:\v" --folder "OMI"') + == r"C:\venv\Scripts\omind" + ) + assert provision._hook_exe_path('omind hook Stop --vault "/v" --folder "OMI"') is None + assert provision._hook_exe_path("python3 /home/x/.claude/hooks/omi-enforce.py") is None + assert provision._hook_exe_path('unbalanced "quote') is None diff --git a/tests/test_store.py b/tests/test_store.py index 2a8c87d..a6a263d 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -884,3 +884,34 @@ def test_notes_are_written_with_lf_on_every_platform(tmp_path: Path) -> None: raw = (tmp_path / "Newlines.md").read_bytes() assert b"\r\n" not in raw assert b"\n" in raw + + +def test_safe_name_resolves_a_title_whose_filename_was_sanitized(tmp_path: Path) -> None: + """The guard names notes by TITLE in its block messages, and a title may hold + characters `filename_for_title` strips. Rejecting the title outright made the + guard's own remediation loop impossible to satisfy: it demanded a recall of + "NEVER offer to end/pause the session …" and then refused the `/`.""" + store = OmiStore(tmp_path) + store.create_note(NoteFields(title="NEVER offer to end/pause the session", summary="s")) + assert (tmp_path / "NEVER offer to end pause the session.md").is_file() + resolved = store.safe_name("NEVER offer to end/pause the session") + assert resolved.name == "NEVER offer to end pause the session.md" + assert store.read_note("NEVER offer to end/pause the session") + + +def test_safe_name_title_fallback_never_enables_traversal(tmp_path: Path) -> None: + """The fallback sanitizes separators away and requires the note to exist, so + it can only ever land on a real note directly inside the OMI dir.""" + store = OmiStore(tmp_path) + (tmp_path.parent / "outside.md").write_text("secret", encoding="utf-8") + for hostile in ("../outside", "../../etc/passwd", "/etc/passwd", "a/../../b"): + with pytest.raises(NoteError): + store.safe_name(hostile) + + +def test_safe_name_still_rejects_a_title_with_no_matching_note(tmp_path: Path) -> None: + """Creates must keep failing loudly — the fallback is a read-side convenience, + not a licence to invent filenames.""" + store = OmiStore(tmp_path) + with pytest.raises(NoteError): + store.safe_name("no/such/note")