diff --git a/BACKLOG.md b/BACKLOG.md index 79265a6..1f6a614 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -79,6 +79,18 @@ _Closed as not planned or rejected. Kept for the record; nothing here is pending ## Done +### Shipped — 2026-10-09 (v10.3.1) + +- [x] **Grok Claude-hook fast path does not count a `use_tool` recall as a consult** ([#472](https://github.com/CryptoJones/omind/issues/472)) — **v10.3.1** — _bug_ — + `omi-guard.sh` unwraps `use_tool` / `CallMcpTool` and camelCase Grok events before the consult + case, and maps `run_terminal_command` onto Bash. A snake_case-only dispatcher on the Claude + harness is translated too, so PostToolUse records the consult. Listing tools still do not clear + the gate. JSON-string `arguments` are parsed in both paths. +- [x] **Commit freshness gate skipped every non-Claude shell tool** ([#473](https://github.com/CryptoJones/omind/issues/473)) — **v10.3.1** — _bug_ — + the gate matched `Bash` only, so Antigravity, Gemini, Hermes, OpenCode and Poolside commits + never needed a fetch. The adapter maps their shell tools onto `Bash`; Antigravity takes the + repo from `run_command`'s `Cwd`. Cross-harness fetch-then-multi-line-commit tests added. + ### Shipped — 2026-10-04 (v10.3.0) - [x] **Grok Build client: setup, doctor, guard payloads, and quickstart** ([#470](https://github.com/CryptoJones/omind/issues/470)) — **v10.3.0** — _feature_ — diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f2aa92..955f37d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,59 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.3.1] - 2026-10-09 + +### Fixed +- **Grok's Claude-hook fast path counts a `use_tool` recall as a consult + ([#472](https://github.com/CryptoJones/omind/issues/472)).** `omi-guard.sh` + reads `.tool_name` only, and v10.3.0's Grok translation runs in the Python + adapter. On the bash fast path a recall arrives as `use_tool` with + `omi__recall-note` nested inside, so it is not a consult, and the gate + denies the recall that would have cleared it. The script now accepts + camelCase, unwraps `use_tool` / `CallMcpTool`, and maps Grok's built-in + names (`run_terminal_command`, `read_file`, …) onto the names it already + classifies. A snake_case-only `use_tool` or built-in name on + `--harness claude` is translated the same way, so PostToolUse records the + consult. Listing tools still do not clear the gate, and vault writes are + still not consults. A plain Claude event is unchanged. A dispatcher's + `arguments` sent as a JSON string is parsed, an empty first payload no + longer hides the real arguments, and a dispatcher with no inner tool name or + an event with no tool is left alone instead of erroring jq. The Python + translator applies the same rules. +- **The commit freshness gate runs in every harness, not just Claude + ([#473](https://github.com/CryptoJones/omind/issues/473)).** Its checks matched the tool name `Bash` only. Antigravity + (`run_command`), Gemini (`run_shell_command`), Hermes (`terminal`), OpenCode + (`bash`) and Poolside (`shell`) send their own names, so a commit with no + fetch went through, and the repo-work and global-config checks were skipped + too. The adapter now maps each harness's shell tool onto `Bash` + (`guard.SHELL_TOOL_NAMES`), and `guard.decide` does the same for a caller + that builds the action itself. The command is read from `command`, + `CommandLine` or `cmd`, as a string or an argv list (Codex `local_shell`), + by both the PreToolUse path and the PostToolUse retraction, so a failed + fetch under any of these names is retracted. +- **The no-jq, no-core last resort in `omi-guard.sh` fails closed for every + shell tool name and for any `use_tool` / `CallMcpTool` dispatcher.** It + matched `Bash` and `run_terminal_command` only. A test pins its list to + `guard.SHELL_TOOL_NAMES`. +- **A dispatcher whose first name field is empty is still unwrapped.** jq's + `//` kept `""`, so `{"tool_name":"","name":"run_terminal_command",…}` stayed + `use_tool` and its command never reached the Bash path. jq now takes the + first non-empty name, as the Python translator does. +- **Antigravity resolves the repo from `run_command`'s own `Cwd`.** It wins + over a top-level `cwd` and the workspace root, so a `git fetch` or + `git commit` run in a repo outside the root is judged against that repo. A + JSON-quoted `Cwd` is decoded, and a relative one is taken from the + workspace root. + +### Tests +- A fetch, then a commit with a multi-line `-m` trailer block, run from a + different repo's cwd, is fresh in every harness that runs the guard + (Claude, Codex, Grok, Antigravity, Gemini, Hermes, OpenCode, Poolside; + MiniMax models run inside these). omind 10.0.2 split that commit at the + newline inside the quotes, lost its `git -C` path, and judged it against + the cwd's repo, so a commit right after a clean fetch was blocked. Later + releases fixed that as a side effect; these tests pin it. + ## [10.3.0] - 2026-10-04 ### Added diff --git a/README.md b/README.md index 1b76154..3857c6a 100644 --- a/README.md +++ b/README.md @@ -671,9 +671,10 @@ under `[mcp_servers.omi]` in `~/.grok/config.toml`. Hooks go in `SessionStart` do accounting, the loop guard, and session bookkeeping. Grok does not inject `SessionStart` stdout, so priming is a managed `~/.grok/rules/omind.md` plus the packaged skill in `~/.grok/skills/omind/`. -Grok also runs Claude Code's hooks and sends them a camelCase payload -(`toolName`, `toolInput`, `sessionId`); the guard accepts that shape and -answers a deny as `{"decision":"deny","reason":…}`. `$GROK_HOME` overrides +Grok also runs Claude Code's hooks. The bash fast path and the Python +adapter accept camelCase (`toolName`, `toolInput`, `sessionId`) and +snake_case, unwrap `use_tool` so an `omi__…` recall counts as a consult, +and answer a deny as `{"decision":"deny","reason":…}`. `$GROK_HOME` overrides `~/.grok`. `omind quickstart --agent grok` prints the same wiring to apply by hand. diff --git a/pyproject.toml b/pyproject.toml index d7d34ad..c5cb632 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "omind" -version = "10.3.0" +version = "10.3.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 825a16a..452f359 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__ = "10.3.0" +__version__ = "10.3.1" diff --git a/src/omind/adapters.py b/src/omind/adapters.py index b8bce8a..55965b5 100644 --- a/src/omind/adapters.py +++ b/src/omind/adapters.py @@ -53,14 +53,13 @@ def _derive_command(event: dict[str, Any], tool_input: dict[str, Any]) -> str: """ # Poolside's ``shell`` tool carries the command line as ``cmd``; # Antigravity uses ``CommandLine``. - for source in ( - event.get("command"), - tool_input.get("command"), - tool_input.get("CommandLine"), - tool_input.get("cmd"), - ): - if isinstance(source, str) and source: - return source + source = event.get("command") + if isinstance(source, str) and source: + return source + # command / CommandLine / cmd, a string or an argv list (Codex local_shell). + shell = guard.shell_command_text(tool_input) + if shell: + return shell for container in (event, tool_input): for key in ("args", "input"): val = container.get(key) @@ -95,6 +94,8 @@ def normalize_action(event: dict[str, Any]) -> dict[str, Any]: tool_input = args tool_input = tool_input if isinstance(tool_input, dict) else {} command = _derive_command(event, tool_input) + if command and tool in guard.SHELL_TOOL_NAMES: + tool = "Bash" file_path = _first_str( tool_input, ("file_path", "path", "TargetFile", "AbsolutePath") ) or _first_str(event, ("file_path", "path", "TargetFile", "AbsolutePath")) diff --git a/src/omind/guard.py b/src/omind/guard.py index 3db8635..bb22c10 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -511,6 +511,28 @@ def _retract_git_freshness(session: str, repo: Path) -> None: path.unlink() +def shell_command_text(tool_input: object) -> str: + """The command a shell tool runs, from any harness's key, or ``""``. + + ``command`` (Claude, Codex, Gemini, Hermes, Grok), ``CommandLine`` + (Antigravity) or ``cmd`` (Poolside, Codex ``exec_command``). An argv list + (Codex ``local_shell``) is joined with spaces. PreToolUse and the + PostToolUse retraction both read it here, so a failed fetch is retracted + under the same key it was granted under (#473). + """ + if not isinstance(tool_input, dict): + return "" + for key in ("command", "CommandLine", "cmd"): + value = tool_input.get(key) + if isinstance(value, str) and value: + return value + if isinstance(value, list): + joined = " ".join(str(part) for part in value if part is not None).strip() + if joined: + return joined + return "" + + def _tool_outcome_failed(tool_response: object) -> bool: """True on an EXPLICIT failure signal only — a harness that reports no outcome at all is trusted (mirrors hooks._extract_outcome's discipline).""" @@ -538,10 +560,9 @@ def record_freshness_outcome(event: dict[str, Any]) -> None: enforced it (2026-08-27 review). This retracts the grant when the outcome says the command failed. Best-effort; never raises.""" try: - if str(event.get("tool_name") or "") != "Bash": + if str(event.get("tool_name") or "") not in SHELL_TOOL_NAMES: return - tool_input = event.get("tool_input") - command = str(tool_input.get("command") or "") if isinstance(tool_input, dict) else "" + command = shell_command_text(event.get("tool_input")) if not command or not _is_freshness_command(command): return if not _tool_outcome_failed(event.get("tool_response")): @@ -1304,6 +1325,24 @@ def clear_all_gates() -> None: #: Hard rules still apply to everything here; they match on command text, which #: none of these tools carry. _GATE_EXEMPT_TOOLS = frozenset({"ToolSearch", "exit", "todo_action"}) +#: Each harness's shell tool. The adapter maps these onto ``Bash`` so every +#: Bash-only check (commit freshness, repo work, global-config mutation) judges +#: them. Before, a commit from Antigravity's ``run_command`` or Gemini's +#: ``run_shell_command`` skipped the freshness gate: it was not named Bash. +SHELL_TOOL_NAMES = frozenset( + { + "Bash", + "bash", # OpenCode + "shell", # Poolside, Codex + "local_shell", # Codex + "exec_command", # Codex + "shell_command", # Codex + "terminal", # Hermes + "run_command", # Antigravity + "run_shell_command", # Gemini CLI + "run_terminal_command", # Grok + } +) _WRITE_TOOLS = frozenset( { "Edit", @@ -4638,6 +4677,10 @@ def decide(action: dict[str, Any], *, git_rules_missing: bool = False) -> Verdic timeout, so an ordinary command never gets there.""" session = str(action.get("session") or "") command = str(action.get("command") or "") + # Every harness's shell tool is Bash to the checks below, whatever built + # the action (#473). The adapter maps it too; this covers direct callers. + if command and str(action.get("tool") or "") in SHELL_TOOL_NAMES: + action = {**action, "tool": "Bash"} with deadline.scope(): try: diff --git a/src/omind/harness.py b/src/omind/harness.py index 723f3e5..8de890a 100644 --- a/src/omind/harness.py +++ b/src/omind/harness.py @@ -397,6 +397,47 @@ def payload_is_grok(event: dict[str, Any]) -> bool: return "toolName" in event or "toolInput" in event +def _agy_cwd(tool_call: object) -> str: + """The ``Cwd`` argument of an Antigravity ``run_command``, or ``""``. + + Antigravity's transcript logs record argument values JSON-encoded, with + literal double quotes around the path. The hook may send either form, so + a quoted value is decoded. + """ + if not isinstance(tool_call, dict): + return "" + args = tool_call.get("args") + value = args.get("Cwd") if isinstance(args, dict) else None + if not isinstance(value, str): + return "" + value = value.strip() + if len(value) >= 2 and value.startswith('"') and value.endswith('"'): + try: + decoded = json.loads(value) + except (ValueError, RecursionError): + return value + return decoded if isinstance(decoded, str) else "" + return value + + +def _first_dict_arg(*values: object) -> dict[str, Any] | None: + """The first non-empty dispatcher argument: an object, or a JSON string of one. + + An OpenAI-style ``use_tool`` sends ``arguments`` as a JSON string. Without + parsing it, the consult target would be the dispatcher wrapper instead of + the note name. ``omi-guard.sh`` parses the same keys in the same order. + """ + for value in values: + if isinstance(value, str): + try: + value = json.loads(value) + except (ValueError, RecursionError): + continue + if isinstance(value, dict) and value: + return dict(value) + return None + + def _translate_grok(event: dict[str, Any]) -> dict[str, Any]: """Map a Grok hook event onto the Claude-shaped event the rest of omind uses.""" out = dict(event) @@ -430,13 +471,13 @@ def _translate_grok(event: dict[str, Any]) -> dict[str, Any]: inner = tool_input.get("tool_name") or tool_input.get("toolName") or tool_input.get("name") if isinstance(inner, str) and inner: tool = inner - nested = ( - tool_input.get("tool_input") - or tool_input.get("arguments") - or tool_input.get("toolInput") + nested = _first_dict_arg( + tool_input.get("tool_input"), + tool_input.get("arguments"), + tool_input.get("toolInput"), ) - if isinstance(nested, dict): - out["tool_input"] = dict(nested) + if nested is not None: + out["tool_input"] = nested out["tool_name"] = tool if tool in _GROK_TOOL_ALIASES: tool = _GROK_TOOL_ALIASES[tool] @@ -450,6 +491,19 @@ def _translate_grok(event: dict[str, Any]) -> dict[str, Any]: return out +def _claude_event_uses_grok_tool(event: dict[str, Any]) -> bool: + """True when a snake_case Claude-hook event is one of Grok's own tool names. + + Grok's Claude-compatible hook sometimes sends ``tool_name`` / ``tool_input`` + and no camelCase keys, so :func:`payload_is_grok` is false. ``use_tool`` + still hides the MCP tool, and ``run_terminal_command`` is not ``Bash``. + A plain Claude event (``Bash``, ``Edit``, ``mcp__omi__…``, ``omi__…``) does + not match, so :func:`translate_event` can return that same object. + """ + tool = event.get("tool_name") + return isinstance(tool, str) and (tool in _GROK_DISPATCHERS or tool in _GROK_TOOL_ALIASES) + + def translate_event(harness: str, event: dict[str, Any]) -> dict[str, Any]: """Map a harness's raw hook event onto the Claude-shaped event the rest of omind consumes (journal, verifier, accounting, guard). Identity for every @@ -464,7 +518,10 @@ def translate_event(harness: str, event: dict[str, Any]) -> dict[str, Any]: - Grok: camelCase ``toolName`` / ``toolInput`` / ``sessionId``, built-in tool names aliased onto Claude's, MCP tools ``__``. A Grok-shaped payload is translated even when the hook was installed as - Claude's, because Grok runs those hooks with its own event shape. + Claude's, because Grok runs those hooks with its own event shape. A + snake_case ``use_tool`` / ``CallMcpTool`` or a Grok built-in name on the + Claude harness is translated too (#472). ``omi-guard.sh`` normalizes the + same shapes before its consult case. """ if not isinstance(event, dict): return event @@ -488,10 +545,21 @@ def translate_event(harness: str, event: dict[str, Any]) -> dict[str, Any]: out["tool_input"] = agy_tool_input if "session_id" not in out and "conversationId" in out: out["session_id"] = out["conversationId"] - if "cwd" not in out: - workspaces = out.get("workspacePaths") - if isinstance(workspaces, list) and workspaces and isinstance(workspaces[0], str): - out["cwd"] = workspaces[0] + workspaces = out.get("workspacePaths") + root = "" + if isinstance(workspaces, list) and workspaces and isinstance(workspaces[0], str): + root = workspaces[0] + # run_command's own Cwd is where the command runs, so it wins over a + # top-level cwd and the workspace root: a `git commit` run in a repo + # outside the root is judged against that repo (#473). A relative Cwd + # is taken from the root, not from wherever this hook process runs. + agy_cwd = _agy_cwd(tool_call) + if agy_cwd: + if root and not Path(agy_cwd).is_absolute(): + agy_cwd = str(Path(root) / agy_cwd) + out["cwd"] = agy_cwd + elif "cwd" not in out and root: + out["cwd"] = root if "prompt" not in out: prompt = agy_last_prompt(out.get("transcriptPath")) if prompt: @@ -511,6 +579,8 @@ def translate_event(harness: str, event: dict[str, Any]) -> dict[str, Any]: ): out["tool_name"] = "mcp__omi__" + tool return out + if harness == "claude" and _claude_event_uses_grok_tool(event): + return _translate_grok(event) if spec.name != "poolside": return event out = dict(event) diff --git a/src/omind/omi-guard.sh b/src/omind/omi-guard.sh index 32625be..aee176a 100644 --- a/src/omind/omi-guard.sh +++ b/src/omind/omi-guard.sh @@ -44,13 +44,70 @@ if ! command -v jq >/dev/null 2>&1; then # No jq AND no working omind core: the policy genuinely couldn't be evaluated. # Fail OPEN for non-Bash (so the host doesn't wedge on every Read/consult) but # CLOSED for Bash (a destructive command must never run unchecked). - if printf '%s' "$input" | grep -q '"tool_name"[[:space:]]*:[[:space:]]*"Bash"'; then + # Every harness's shell tool (guard.SHELL_TOOL_NAMES), camelCase or + # snake_case, and any use_tool / CallMcpTool dispatcher (it can wrap a + # shell call) is a Bash command here. With neither jq nor the core it must + # not run unchecked. + if printf '%s' "$input" | grep -Eq '"(tool_name|toolName)"[[:space:]]*:[[:space:]]*"(Bash|bash|shell|local_shell|exec_command|shell_command|terminal|run_command|run_shell_command|run_terminal_command|use_tool|CallMcpTool)"'; then printf 'omi-guard: omind core unreachable too — BLOCKING this Bash command (fail-closed).\n' >&2 exit 2 fi exit 0 fi +# Grok Build runs this Claude hook. A camelCase event, a use_tool / CallMcpTool +# dispatcher, and Grok's built-in names have to look like the Claude events the +# case below already classifies. Otherwise a recall arrives as use_tool, the +# gate does not treat it as a consult, and the recall that would clear the gate +# is itself denied (#472). A plain Claude event is unchanged. A jq failure +# keeps the raw event rather than dropping it. +normalized="$(printf '%s' "$input" | jq -c ' + def filled(k): (.[k] | type) == "string" and (.[k] | length) > 0; + if (filled("tool_name") | not) and filled("toolName") then .tool_name = .toolName else . end + | if ((.tool_input | type) != "object") and ((.toolInput | type) == "object") then .tool_input = .toolInput else . end + | if (filled("session_id") | not) and filled("sessionId") then .session_id = .sessionId else . end + | if (filled("transcript_path") | not) and filled("transcriptPath") then .transcript_path = .transcriptPath else . end + | if (filled("prompt") | not) and filled("userPrompt") then .prompt = .userPrompt + elif (filled("prompt") | not) and filled("message") then .prompt = .message + else . end + | (.tool_name | if type == "string" then . else "" end) as $tool + | if ($tool == "use_tool" or $tool == "CallMcpTool") then + (if (.tool_input | type) == "object" then .tool_input else {} end) as $outer + # First NON-EMPTY name, as the Python `or` chain picks. The jq `//` + # keeps "", which left `{"tool_name":"","name":"run_terminal_command"}` + # wrapped and its command off the Bash path. + | ([$outer.tool_name, $outer.toolName, $outer.name] + | map(select(type == "string" and length > 0)) | first // "") as $inner + | if ($inner | type) == "string" and ($inner | length) > 0 then + .tool_name = $inner + # The nested arguments may arrive as a JSON string (an OpenAI-style + # dispatcher). Parse it, so the consult target is the note name and + # not the dispatcher wrapper. + | .tool_input = ( + [$outer.tool_input, $outer.arguments, $outer.toolInput] + | map(if type == "string" then (fromjson? // null) else . end) + | map(select(type == "object" and length > 0)) + | first // $outer + ) + else . end + else . end + | (.tool_name | if type == "string" then . else "" end) as $t + | if $t == "" then . + elif $t == "run_terminal_command" then .tool_name = "Bash" + elif $t == "read_file" then .tool_name = "Read" + elif $t == "search_replace" then .tool_name = "Edit" + elif $t == "write" then .tool_name = "Write" + elif $t == "grep" then .tool_name = "Grep" + elif $t == "list_dir" then .tool_name = "LS" + elif $t == "web_search" then .tool_name = "WebSearch" + elif $t == "spawn_subagent" then .tool_name = "Task" + elif ($t | startswith("omi__")) or (($t | contains("__")) and ($t | startswith("mcp__") | not)) then .tool_name = "mcp__" + $t + elif ($t | startswith("omi_")) then .tool_name = "mcp__omi__" + $t[4:] + elif ($t == "search-vault" or $t == "recall-note" or $t == "read-note" or $t == "create-note" or $t == "edit-note" or $t == "list-notes" or $t == "help") then .tool_name = "mcp__omi__" + $t + else . end +' 2>/dev/null)" || normalized="" +[ -n "$normalized" ] && input="$normalized" + tool="$(printf '%s' "$input" | jq -r '.tool_name // empty' 2>/dev/null)" sid="$(printf '%s' "$input" | jq -r '.session_id // empty' 2>/dev/null | tr -cd 'A-Za-z0-9._-')" [ -z "$sid" ] && sid="nosid" diff --git a/tests/test_adapters.py b/tests/test_adapters.py index ed3d90d..1a4c2cc 100644 --- a/tests/test_adapters.py +++ b/tests/test_adapters.py @@ -679,3 +679,125 @@ def test_run_adapter_last_resort_renders_a_literal_allow( data = json.loads(out) assert data.get("allow", True) is True assert data.get("decision", "allow") == "allow" + + +# -- fetch-then-commit freshness across harnesses ---------------------------- + + +def _bare_repo(root: Path, name: str) -> Path: + repo = root / name + (repo / ".git").mkdir(parents=True) + (repo / ".git" / "HEAD").write_text("ref: refs/heads/main\n", encoding="utf-8") + (repo / ".git" / "config").write_text( + '[remote "origin"]\n\turl = https://example.invalid/r.git\n', encoding="utf-8" + ) + return repo.resolve() + + +def _shell_event(harness: str, command: str, session: str, cwd: Path, root: Path) -> dict[str, Any]: + """One shell call in ``harness``'s native hook shape, run in ``cwd``. + ``root`` is the session's workspace root, which differs from ``cwd``.""" + if harness == "grok": + return { + "hookEventName": "pre_tool_use", + "toolName": "run_terminal_command", + "toolInput": {"command": command}, + "sessionId": session, + "cwd": str(cwd), + } + if harness == "agy": + return { + "toolCall": {"name": "run_command", "args": {"CommandLine": command, "Cwd": str(cwd)}}, + "conversationId": session, + "workspacePaths": [str(root)], + } + return { + "hook_event_name": "PreToolUse", + "tool_name": _SHELL_TOOL.get(harness, "Bash"), + "tool_input": {"cmd" if harness == "poolside" else "command": command}, + "session_id": session, + "cwd": str(cwd), + } + + +#: The shell tool name each harness sends; the rest send ``Bash``. +_SHELL_TOOL = { + "gemini": "run_shell_command", + "hermes": "terminal", + "opencode": "bash", + "poolside": "shell", +} + + +def _denied(code: int, out: str) -> bool: + """A deny in any harness's block format: exit 2, or a deny/block JSON.""" + if code == 2: + return True + try: + body = json.loads(out) if out.strip() else {} + except ValueError: + return False + text = json.dumps(body) + return '"deny"' in text or '"block"' in text or body.get("allow") is False + + +@pytest.mark.parametrize( + "harness", ["claude", "codex", "grok", "agy", "gemini", "hermes", "opencode", "poolside"] +) +@pytest.mark.parametrize("dash_c", [False, True]) +def test_fetch_then_multiline_commit_is_fresh_in_every_harness( + harness: str, dash_c: bool, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A clean standalone fetch frees the next commit in the same repo, in + every harness that runs the guard (MiniMax models run inside these). The + commit message spans lines, the session's workspace root is another repo, + and the repo comes from `git -C` or from the shell's cwd. Each harness + sends its own shell tool name, so the commit before the fetch must be + blocked too: a non-`Bash` name used to skip the freshness gate.""" + root = _bare_repo(tmp_path, "workspace") + target = _bare_repo(tmp_path, "todo") + session = f"fresh-{harness}-{int(dash_c)}" + guard.clear_gate(session) + guard.record_consult(session, kind="read", target=guard.GIT_RULES_NOTE, relevant=True) + prefix = f'git -C "{target}"' if dash_c else "git" + cwd = root if dash_c else target + + last = {"text": ""} + + def run(command: str) -> bool: + event = _shell_event(harness, command, session, cwd, root) + code = adapters.run_adapter(io.StringIO(json.dumps(event)), harness=harness) + captured = capsys.readouterr() + last["text"] = captured.out + captured.err + return not _denied(code, captured.out) + + commit = f'{prefix} commit -q -am "Add #15" -m "Co-Authored-By: A \nSession: x"' + assert not run(commit), "a commit before any fetch must be blocked" + assert "freshness" in last["text"], "blocked by the freshness gate, not another rule" + assert run(f"{prefix} fetch --all --prune") + assert str(target) in guard._fresh_repos(session) + assert run(commit), "the commit after a clean fetch must pass" + guard.clear_gate(session) + + +@pytest.mark.parametrize( + ("tool", "tool_input"), + [ + ("exec_command", {"cmd": "git commit -am x"}), + ("local_shell", {"command": ["git", "commit", "-am", "x"]}), + ("shell_command", {"command": "git commit -am x"}), + ], +) +def test_codex_shell_tools_reach_the_bash_path(tool: str, tool_input: dict[str, Any]) -> None: + """#473: Codex's own shell tools, with `cmd` or an argv list, normalize + to a Bash action carrying the command.""" + action = adapters.normalize_action({"tool_name": tool, "tool_input": tool_input}) + assert action["tool"] == "Bash" + assert action["command"] == "git commit -am x" + + +def test_non_shell_tool_with_a_command_key_is_not_bash() -> None: + action = adapters.normalize_action( + {"tool_name": "read_file", "tool_input": {"command": "x", "path": "/a"}} + ) + assert action["tool"] == "read_file" diff --git a/tests/test_grok_client.py b/tests/test_grok_client.py index 5852ae4..d4ac5d3 100644 --- a/tests/test_grok_client.py +++ b/tests/test_grok_client.py @@ -105,6 +105,46 @@ def test_grok_dispatcher_lifts_inner_mcp_tool() -> None: assert event["tool_input"]["name"].startswith("Operational Rules") +@pytest.mark.parametrize( + "tool_input", + [ + {"name": "omi__recall-note", "arguments": json.dumps({"name": "Rules"})}, + {"toolName": "omi__recall-note", "tool_input": {}, "arguments": {"name": "Rules"}}, + ], +) +def test_grok_dispatcher_parses_string_and_skips_empty_args(tool_input: dict[str, object]) -> None: + """#472: JSON-string `arguments` are parsed and an empty first payload is + skipped, the same as omi-guard.sh, so the consult target is the note.""" + event = harness.translate_event("grok", {"toolName": "use_tool", "toolInput": tool_input}) + assert event["tool_name"] == "mcp__omi__recall-note" + assert event["tool_input"] == {"name": "Rules"} + + +def test_claude_harness_translates_snake_case_grok_tools() -> None: + """#472: Grok's Claude-compatible hook can send snake_case only. On + `--harness claude` a `use_tool` or a Grok built-in name is still mapped, + so PostToolUse records the consult and the shell command is Bash.""" + recall = harness.translate_event( + "claude", + { + "tool_name": "use_tool", + "tool_input": {"tool_name": "omi__recall-note", "tool_input": {"name": "Rules"}}, + }, + ) + assert recall["tool_name"] == "mcp__omi__recall-note" + assert recall["tool_input"] == {"name": "Rules"} + shell = harness.translate_event( + "claude", {"tool_name": "run_terminal_command", "tool_input": {"command": "ls"}} + ) + assert shell["tool_name"] == "Bash" + + +@pytest.mark.parametrize("tool", ["Bash", "Edit", "mcp__omi__recall-note", "Write"]) +def test_claude_harness_leaves_plain_claude_events_alone(tool: str) -> None: + event = {"tool_name": tool, "tool_input": {"command": "ls"}, "session_id": "s"} + assert harness.translate_event("claude", event) is event + + @pytest.fixture def grok_home(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> Path: root = tmp_path / "grok-home" diff --git a/tests/test_guard.py b/tests/test_guard.py index 4da4084..adc255d 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -1123,6 +1123,21 @@ def run(event: dict[str, object]) -> int: assert run(_BASH_EVENT) == 2 # destructive command must not run unchecked edit_event = {"tool_name": "Edit", "session_id": "h", "tool_input": {"file_path": "/x"}} assert run(edit_event) == 0 # a non-Bash tool must not wedge the host + # #473: every harness's shell tool, and a dispatcher that may wrap one. + for name in sorted(guard.SHELL_TOOL_NAMES | {"use_tool", "CallMcpTool"}): + command = {"command": "rm -rf /tmp/x"} + assert run({"tool_name": name, "tool_input": command}) == 2, name + assert run({"toolName": name, "toolInput": command}) == 2, name + + +def test_hook_no_jq_fallback_lists_every_shell_tool() -> None: + """The no-jq, no-core grep in omi-guard.sh is a hand-written copy of + guard.SHELL_TOOL_NAMES. Fail when the two drift apart.""" + src = importlib.resources.files("omind").joinpath("omi-guard.sh").read_text(encoding="utf-8") + match = re.search(r'\[\[:space:\]\]\*"\(([^)]*)\)"\'; then', src) + assert match is not None + names = set(match.group(1).split("|")) + assert names == set(guard.SHELL_TOOL_NAMES) | {"use_tool", "CallMcpTool"} def _read_event(omi: Path, name: str, sid: str) -> dict[str, object]: @@ -1317,6 +1332,45 @@ def test_dash_c_fetch_attributes_freshness_to_the_target_repo( guard.clear_gate("dashc") +def test_multiline_commit_message_keeps_the_dash_c_repo(tmp_path: Path) -> None: + """A commit whose quoted `-m` spans lines (a trailer block) still resolves + to its `git -C` repo. omind 10.0.2 split the command at the newline inside + the quotes, lost the `-C`, and judged the commit against the event cwd's + repo. That repo was never fetched, so a commit right after a clean fetch + was blocked. Seen in Claude (Co-Authored-By trailers) and expected from + any agent that writes trailers (Grok, Codex, Antigravity, MiniMax models).""" + vault = _mk_repo(tmp_path, "vault") + target = _mk_repo(tmp_path, "todo") + session = "mlcommit" + guard.clear_gate(session) + guard.record_consult(session, kind="read", target=guard.GIT_RULES_NOTE, relevant=True) + fetch = guard.decide( + { + "tool": "Bash", + "command": f'git -C "{target}" fetch --all --prune', + "session": session, + "cwd": str(vault), + } + ) + assert fetch.allow + assert str(target) in guard._fresh_repos(session) + message = "Co-Authored-By: Agent \nAgent-Session: https://x.invalid/s" + commit = { + "tool": "Bash", + "command": f'git -C "{target}" commit -q -am "Add #15" -m "{message}"', + "session": session, + "cwd": str(vault), + } + assert guard._repo_root_for_action(commit) == target + verdict = guard.decide(commit) + assert verdict.allow, verdict.reason + # The vault repo was never fetched: a multi-line commit there is still stale. + stale = guard.decide({**commit, "command": commit["command"].replace(str(target), str(vault))}) + assert not stale.allow + assert stale.rule_id == "repo-work-fresh-base" + guard.clear_gate(session) + + def test_dash_c_parsing_edge_cases_fall_back_to_cwd( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -1436,6 +1490,137 @@ def test_hook_forwards_the_event_cwd_to_the_core(tmp_path: Path) -> None: assert json.loads(capture.read_text(encoding="utf-8"))["cwd"] == "/w/tree" +_RULES = "Operational Rules - Git Repos and Secrets" + + +@pytest.mark.skipif(not _HOOK_TESTABLE, reason="omi-guard.sh is a POSIX bash+jq adapter") +@pytest.mark.parametrize( + "event", + [ + # Grok's camelCase dispatcher with an object payload. + { + "toolName": "use_tool", + "sessionId": "g", + "toolInput": {"tool_name": "omi__recall-note", "tool_input": {"name": _RULES}}, + }, + # snake_case dispatcher on the Claude-compatible hook, JSON-string args. + { + "tool_name": "use_tool", + "session_id": "g", + "tool_input": {"name": "omi__recall-note", "arguments": json.dumps({"name": _RULES})}, + }, + # An empty first payload does not hide the real arguments. + { + "tool_name": "CallMcpTool", + "session_id": "g", + "tool_input": { + "toolName": "recall-note", + "tool_input": {}, + "arguments": {"name": _RULES}, + }, + }, + ], +) +def test_hook_counts_a_grok_dispatched_recall_as_a_consult( + tmp_path: Path, event: dict[str, object] +) -> None: + """#472: a recall wrapped in `use_tool` / `CallMcpTool` reaches the core as + an OMI consult of the note it names, so it clears the gate.""" + fake, capture = _capturing_omind(tmp_path) + hook = _render_hook(tmp_path, str(fake)) + assert _run_hook(hook, event) == 0 + sent = json.loads(capture.read_text(encoding="utf-8")) + assert sent["is_omi_consult"] is True + assert sent["tool"] == "mcp__omi__recall-note" + assert sent["consult_target"] == _RULES + assert sent["session"] == "g" + + +@pytest.mark.skipif(not _HOOK_TESTABLE, reason="omi-guard.sh is a POSIX bash+jq adapter") +def test_hook_judges_grok_terminal_command_as_bash(tmp_path: Path) -> None: + """#472: Grok's `run_terminal_command` goes down the fail-closed Bash path + with its command and cwd, so the hard rules and freshness see it.""" + fake, capture = _capturing_omind(tmp_path) + hook = _render_hook(tmp_path, str(fake)) + event = { + "toolName": "run_terminal_command", + "sessionId": "g", + "cwd": "/w/tree", + "toolInput": {"command": 'git -C "/w/tree" fetch --all --prune'}, + } + assert _run_hook(hook, event) == 0 + sent = json.loads(capture.read_text(encoding="utf-8")) + assert sent["tool"] == "Bash" + assert sent["command"] == 'git -C "/w/tree" fetch --all --prune' + assert sent["cwd"] == "/w/tree" + + +@pytest.mark.skipif(not _HOOK_TESTABLE, reason="omi-guard.sh is a POSIX bash+jq adapter") +@pytest.mark.parametrize( + "event", + [ + {"tool_name": "use_tool", "session_id": "g", "tool_input": "omi__recall-note"}, + {"tool_name": "use_tool", "session_id": "g", "tool_input": {}}, + {"session_id": "g", "tool_input": {"command": "ls"}}, + ], +) +def test_hook_odd_dispatcher_shapes_are_not_consults( + tmp_path: Path, event: dict[str, object] +) -> None: + """A dispatcher with no inner tool name, or an event with no tool at all, + is not a consult. The normalizer leaves it alone instead of erroring.""" + fake, capture = _capturing_omind(tmp_path) + hook = _render_hook(tmp_path, str(fake)) + assert _run_hook(hook, event) == 0 # the capturing core allows + sent = json.loads(capture.read_text(encoding="utf-8")) + assert sent["is_omi_consult"] is False + + +_DISPATCHED = "gh repo delete a/b" + + +@pytest.mark.skipif(not _HOOK_TESTABLE, reason="omi-guard.sh is a POSIX bash+jq adapter") +@pytest.mark.parametrize( + "tool_input", + [ + # An empty first name: jq's `//` used to keep "" and skip the unwrap. + {"tool_name": "", "name": "run_terminal_command", "arguments": {"command": _DISPATCHED}}, + {"name": "run_terminal_command", "arguments": json.dumps({"command": _DISPATCHED})}, + ], +) +def test_hook_unwraps_a_dispatched_shell_command_onto_bash( + tmp_path: Path, tool_input: dict[str, object] +) -> None: + """A shell command wrapped in `use_tool` reaches the core's Bash path with + its command, so the hard rules see it. Python unwraps the same way.""" + from omind import harness + + fake, capture = _capturing_omind(tmp_path) + hook = _render_hook(tmp_path, str(fake)) + event = {"tool_name": "use_tool", "session_id": "g", "tool_input": tool_input} + assert _run_hook(hook, event) == 0 + sent = json.loads(capture.read_text(encoding="utf-8")) + assert (sent["tool"], sent["command"]) == ("Bash", _DISPATCHED) + translated = harness.translate_event("grok", {**event, "toolName": "use_tool"}) + assert translated["tool_name"] == "Bash" + assert translated["tool_input"]["command"] == _DISPATCHED + + +def test_decide_treats_every_shell_tool_as_bash(tmp_path: Path) -> None: + """#473: a caller that builds the action itself (no adapter) with a + harness's shell tool name still gets the commit freshness gate.""" + repo = _mk_repo(tmp_path, "r") + session = "decide-shell" + guard.clear_gate(session) + guard.record_consult(session, kind="read", target=guard.GIT_RULES_NOTE, relevant=True) + for tool in sorted(guard.SHELL_TOOL_NAMES): + verdict = guard.decide( + {"tool": tool, "command": f'git -C "{repo}" commit -am x', "session": session} + ) + assert verdict.rule_id == "repo-work-fresh-base", tool + guard.clear_gate(session) + + @pytest.mark.skipif(not _HOOK_TESTABLE, reason="omi-guard-hermes.sh is a POSIX bash+jq adapter") def test_hermes_hook_forwards_the_event_cwd_to_the_core(tmp_path: Path) -> None: """Hermes' shell-hook payload carries `cwd`; its adapter forwards it too.""" @@ -2430,6 +2615,37 @@ def test_record_freshness_outcome_retracts_for_dash_c_repo(tmp_path: Path) -> No assert not guard._git_fresh_for_repo("sess-f", repo) +@pytest.mark.parametrize( + ("tool", "key"), + [ + ("run_command", "CommandLine"), # Antigravity, untranslated + ("shell", "cmd"), # Poolside, untranslated + ("run_shell_command", "command"), # Gemini + ("terminal", "command"), # Hermes + ("run_terminal_command", "command"), # Grok + ("exec_command", "cmd"), # Codex + ], +) +def test_failed_fetch_is_retracted_for_every_shell_tool( + tmp_path: Path, tool: str, key: str +) -> None: + """#473: a fetch that failed under a non-Bash shell tool name, with the + command under that harness's own key, does not leave the repo fresh.""" + repo = _mk_repo(tmp_path, "subrepo") + session = f"retract-{tool}" + guard.begin_turn(session, "task") + guard._record_git_freshness(session, repo, f'git -C "{repo}" fetch') + guard.record_freshness_outcome( + { + "session_id": session, + "tool_name": tool, + "tool_input": {key: f'git -C "{repo}" fetch --all --prune'}, + "tool_response": {"exit_code": 1}, + } + ) + assert not guard._git_fresh_for_repo(session, repo) + + # --------------------------------------------------------------------------- # #296 — consult continuity: continuation prompts, retry carry, action budget # --------------------------------------------------------------------------- diff --git a/tests/test_harness.py b/tests/test_harness.py index caed5ca..7aecc75 100644 --- a/tests/test_harness.py +++ b/tests/test_harness.py @@ -8,6 +8,8 @@ import json from pathlib import Path +import pytest + from omind import guard, harness @@ -364,3 +366,37 @@ def test_translate_event_recovers_agy_prompt_from_transcript(tmp_path: Path) -> assert harness.agy_last_prompt(None) == "" assert harness.agy_last_prompt(str(tmp_path / "absent.jsonl")) == "" + + +@pytest.mark.parametrize("quoted", [False, True]) +def test_agy_run_command_cwd_beats_the_workspace_root(tmp_path: Path, quoted: bool) -> None: + """Antigravity's run_command runs in its own `Cwd`, plain or JSON-quoted. + The workspace root is only the fallback, so a commit in another repo is + judged against that repo. Real absolute paths: on Windows `/w` is + drive-relative, not absolute.""" + repo, root = str(tmp_path / "repo"), str(tmp_path / "root") + cwd = json.dumps(repo) if quoted else repo + event = { + "toolCall": {"name": "run_command", "args": {"CommandLine": "git commit", "Cwd": cwd}}, + "conversationId": "a", + "workspacePaths": [root], + } + assert harness.translate_event("agy", event)["cwd"] == repo + del event["toolCall"]["args"]["Cwd"] + assert harness.translate_event("agy", event)["cwd"] == root + + +def test_agy_cwd_beats_a_top_level_cwd_and_resolves_relative(tmp_path: Path) -> None: + repo, root, other = (str(tmp_path / name) for name in ("repo", "root", "other")) + + def cwd_of(args: dict[str, str], **top: str) -> str: + event = { + "toolCall": {"name": "run_command", "args": {"CommandLine": "git commit", **args}}, + "workspacePaths": [root], + **top, + } + return str(harness.translate_event("agy", event)["cwd"]) + + assert cwd_of({"Cwd": repo}, cwd=root) == repo + assert Path(cwd_of({"Cwd": "todo"})) == Path(root) / "todo" + assert cwd_of({}, cwd=other) == other diff --git a/uv.lock b/uv.lock index bef32e6..62e0bef 100644 --- a/uv.lock +++ b/uv.lock @@ -2364,7 +2364,7 @@ wheels = [ [[package]] name = "omind" -version = "10.3.0" +version = "10.3.1" source = { editable = "." } dependencies = [ { name = "cryptography" },