diff --git a/BACKLOG.md b/BACKLOG.md index 5f93b5e..0b338f8 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -38,11 +38,11 @@ hardening, or docs, not defects._ independently of policy content. The detector was covered only indirectly (via a seed rule that a policy change had already forced an edit to); the other two were not covered at all. -- [ ] **One failing PostToolUse side effect silently cancels the rest** ([#204](https://github.com/CryptoJones/omind/issues/204)) — _hardening_ — - found while writing the #189 test. The four side effects share a single `try/except`, - so a token-accounting failure disables the violation detector and the consult verifier - for that call, with exit code 0 and only a `hook-failures.log` breadcrumb. Give each its - own handler. Same shape in the `Stop` branch. +- [x] **One failing PostToolUse side effect silently cancels the rest** ([#204](https://github.com/CryptoJones/omind/issues/204)) — _hardening_ — + found while writing the #189 test. Each side effect now runs isolated through + `hooks._best_effort`, which leaves a breadcrumb naming which one failed, so + `hook-failures.log` tells "it ran and failed" from "it never ran". The `Stop` branch + is isolated the same way. - [x] **Document that `omind serve` is an unauthenticated destructive API (localhost-only by design)** ([#190](https://github.com/CryptoJones/omind/issues/190)) — _docs_ — `docs/serve.md` states the risk model: every route an unauthenticated caller reaches, what already protects you and why, how to expose the port safely, and what to check if diff --git a/CHANGELOG.md b/CHANGELOG.md index 0694cf1..771af39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ 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). +## [7.0.0] - 2026-08-02 + +_The last of the 2026-08-01 review and 2026-08-02 comparison backlogs, released +together rather than as another run of point versions._ + +### Fixed +- **One failing `PostToolUse` side effect no longer cancels the rest** + ([#204](https://github.com/CryptoJones/omind/issues/204), found while writing + the #189 test). `hooks.run_hook` ran four independent subsystems — the loop + guard, token accounting, the Layer E violation detector, and the Layer C + consult verifier — inside a **single** `try/except`. A failure in the first, + which is token accounting, the most fragile and least important of the four, + silently skipped the enforcement detector and the verifier behind it. The hook + still returned 0, by design, so the only trace was an unlabelled breadcrumb. + + Each side effect now runs isolated through `hooks._best_effort`, which records + a breadcrumb naming *which* one failed — so `hook-failures.log` distinguishes + "it ran and failed" from "it never ran". The `Stop` branch had the same shape, + where a transcript-parsing failure meant the loop guard was never consulted; + it is isolated too. + ## [6.6.0] - 2026-08-02 ### Added diff --git a/pyproject.toml b/pyproject.toml index c3463ac..f2533ae 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "omind" -version = "6.6.0" +version = "7.0.0" 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 af2c7b5..0909da6 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__ = "6.6.0" +__version__ = "7.0.0" diff --git a/src/omind/hooks.py b/src/omind/hooks.py index e735052..0f34a8f 100644 --- a/src/omind/hooks.py +++ b/src/omind/hooks.py @@ -34,12 +34,16 @@ import json import os import sys +from collections.abc import Callable from datetime import datetime from pathlib import Path -from typing import Any, TextIO +from typing import Any, TextIO, TypeVar from omind import filelock, paths +#: Return type of one isolated hook side effect (see :func:`_best_effort`). +_T = TypeVar("_T") + HOOK_MARKER = "omind hook" # substring used by provision.py to find our entries HANDLED_EVENTS = ("PostToolUse", "Stop", "SessionStart") #: Hermes Agent has no SessionStart hook; it fires ``pre_llm_call`` before every @@ -107,6 +111,24 @@ def _record_failure(context: str, exc: BaseException) -> None: return +def _best_effort(label: str, call: Callable[[], _T]) -> _T | None: + """Run one hook side effect, absorbing its failure with a labelled breadcrumb. + + The hook branches below run several *independent* subsystems in sequence — + accounting, the violation detector, the consult verifier. Sharing one + ``try/except`` meant a failure in the first silently cancelled every later + one: token accounting going down took the enforcement detector with it, exit + code 0, no symptom (#204). Each one now degrades only itself, and the + breadcrumb names *which*, so `hook-failures.log` distinguishes "it ran and + failed" from "it never ran". + """ + try: + return call() + except Exception as exc: + _record_failure(label, exc) + return None + + def _now(now: datetime | None) -> datetime: return now if now is not None else datetime.now() @@ -676,37 +698,58 @@ def run_hook( if line: append_entry(omi_dir, line) if event_name == "Stop": - from omind import ai_usage - - ai_usage.record_session_transcript( - omi_dir, - event.get("transcript_path"), - session_id=str(event.get("session_id") or ""), + from omind import ai_usage, loopguard + + # Isolated for the same reason as PostToolUse (#204): transcript + # parsing is the failure-prone half, and it must not stop the loop + # guard below from being consulted at all. + _best_effort( + "Stop/ai_usage.record_session_transcript", + lambda: ai_usage.record_session_transcript( + omi_dir, + event.get("transcript_path"), + session_id=str(event.get("session_id") or ""), + ), ) # Loop guard: while an autonomous loop is ARMED, refuse the stop so the # agent keeps working (operator switch — `omind loop arm/disarm`). Fails # open to allowing the stop on any error (a broken guard must never trap). - from omind import loopguard - - blocked, reason = loopguard.register_block(session=event.get("session_id")) + blocked, reason = _best_effort( + "Stop/loopguard.register_block", + lambda: loopguard.register_block(session=event.get("session_id")), + ) or (False, "") if blocked: sink = stdout if stdout is not None else sys.stdout sink.write(json.dumps({"decision": "block", "reason": reason}) + "\n") return 0 if event_name == "PostToolUse": - # Real work happened: reset the loop guard's no-work spin counter. - from omind import loopguard + from omind import ai_usage, compliance, loopguard, verify - loopguard.reset(session=event.get("session_id")) + # Four independent subsystems. Each is isolated (#204) so a failure + # in one — accounting is the most fragile, and the least important — + # cannot silently disable the enforcement detector behind it. + # + # Real work happened: reset the loop guard's no-work spin counter. + _best_effort( + "PostToolUse/loopguard.reset", + lambda: loopguard.reset(session=event.get("session_id")), + ) + _best_effort( + "PostToolUse/ai_usage.record_mcp_response", + lambda: ai_usage.record_mcp_response(omi_dir, event), + ) # Layer E: scan the command that actually ran against the policy and # record any rule match into the compliance log (the learning corpus). - from omind import ai_usage, compliance, verify - - ai_usage.record_mcp_response(omi_dir, event) - compliance.record_post_tool(event) + _best_effort( + "PostToolUse/compliance.record_post_tool", + lambda: compliance.record_post_tool(event), + ) # Layer C: if this action was an OMI consult, judge its relevance to # the turn's task (off the synchronous PreToolUse hot path). - verify.verify_consult(event, omi_dir) + _best_effort( + "PostToolUse/verify.verify_consult", + lambda: verify.verify_consult(event, omi_dir), + ) except Exception as exc: _record_failure(f"run_hook({event_name}, {omi_dir})", exc) return 0 diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 56544a7..02f12c9 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -279,14 +279,14 @@ def record(*args: object, **kwargs: object) -> None: assert event in called["verify"]["args"] # type: ignore[operator] -def test_post_tool_use_side_effects_are_independent_of_the_policy( +def test_one_failing_post_tool_use_side_effect_does_not_cancel_the_others( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """One failing side effect must not silently cancel the others. + """A failure in one subsystem must degrade only that subsystem (#204). - They run in sequence inside a single try/except, so an exception in an - early one skipped every later one. The hook still returns 0 either way, - which is exactly what makes this invisible without a test. + They used to share a single try/except, so token accounting going down took + the violation detector and the consult verifier with it — exit code 0, no + symptom, enforcement silently off. """ from omind import ai_usage, compliance, verify @@ -302,8 +302,31 @@ def boom(*_a: object, **_k: object) -> None: stdin = io.StringIO('{"hook_event_name": "PostToolUse", "tool_name": "Bash"}') assert hooks.run_hook("PostToolUse", tmp_path, stdin=stdin) == 0 - assert seen == [] # KNOWN: an early failure cancels the rest — see #204 - assert hooks.failure_log_path().exists() # but it does leave a breadcrumb + assert seen == ["compliance", "verify"] # the detector still ran + # And the failure is attributable, not just swallowed. + trace = hooks.failure_log_path().read_text(encoding="utf-8") + assert "PostToolUse/ai_usage.record_mcp_response" in trace + + +def test_a_failing_stop_transcript_still_consults_the_loop_guard( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Same isolation on the Stop branch (#204).""" + from omind import ai_usage, loopguard + + def boom(*_a: object, **_k: object) -> None: + raise RuntimeError("unparseable transcript") + + consulted: list[str] = [] + monkeypatch.setattr(ai_usage, "record_session_transcript", boom) + monkeypatch.setattr( + loopguard, + "register_block", + lambda **_k: (consulted.append("loopguard"), (False, ""))[1], + ) + stdin = io.StringIO('{"hook_event_name": "Stop", "session_id": "s"}') + assert hooks.run_hook("Stop", tmp_path, stdin=stdin) == 0 + assert consulted == ["loopguard"] def test_run_hook_session_start_emits_context_no_journal(tmp_path: Path) -> None: diff --git a/uv.lock b/uv.lock index 0c8c9f2..3ff003f 100644 --- a/uv.lock +++ b/uv.lock @@ -2354,7 +2354,7 @@ wheels = [ [[package]] name = "omind" -version = "6.6.0" +version = "7.0.0" source = { editable = "." } dependencies = [ { name = "cryptography" },