mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(grok): close the Claude-parity divergences (reasoning, subagents, web, bash-guard)
Bring the grok CLI to parity with the Claude path on the four deliberate
differences:
- Reasoning: drop the per-role `--effort low` default — Claude sets no per-role
thinking budget, so grok now uses the model default for every role. The
fleet-wide ROBOCO_GROK_REASONING_EFFORT override stays as a cost lever. (This
also un-caps intake-draft quality, the one that actually mattered.)
- Subagents: the intake interviewer may now fan out to subagents (parity with the
Claude intake's `Task` allowance); every other role still has `Agent` removed.
- Web: `--disable-web-search` for every role — no agent gets direct web (Claude's
tool set has none either); the roles that get web reach it through the gated
roboco-search MCP, unaffected.
- Bash command filtering: full parity, split by deny semantics. Verified live that
a grok PreToolUse hook deny CANCELS the run, while native `--deny` denies
GRACEFULLY (the agent gets a permission error and recovers). So:
* git network/branch/history ops -> native `--deny` (operational reflex; the
agent must recover, not drop the task). Expanded to the full bash-guard set.
* credential-exfil / identity-forgery / internal-API / env-dump patterns ->
the SAME bash-guard the Claude path runs, wired as a grok PreToolUse hook
(ROBOCO_GUARD_SKIP_GIT=1 so it leaves git to `--deny`). A hard cancel is the
right response there — no legitimate agent reads ~/.netrc or forges an
X-Agent-ID. One tolerance line (accept grok's camelCase `toolInput`) makes
the one tested script guard both runtimes; +5 grok cases (50/50 green).
Also cleaned stale internal task-number / smoke labels out of bash-guard-hook.sh.
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import tomllib
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
@@ -91,20 +92,76 @@ def test_main_pm_keeps_shell_but_denies_git(monkeypatch: pytest.MonkeyPatch) ->
|
||||
assert "run_terminal_cmd" not in dis # PM keeps a shell
|
||||
assert "search_replace" in dis # but does not write code
|
||||
assert "Bash(git push*)" in args # raw git mutation denied
|
||||
assert args[args.index("--effort") + 1] == "low"
|
||||
# Parity with Claude: model-default reasoning for every role, no per-role cut.
|
||||
assert "--effort" not in args
|
||||
|
||||
|
||||
def test_effort_override(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
def test_prompter_allows_subagents_but_no_shell_or_edit(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.delenv("ROBOCO_GROK_REASONING_EFFORT", raising=False)
|
||||
dis = _disallowed(gc.grok_cli_args_for_role("prompter"))
|
||||
# The intake interviewer may fan out to subagents (parity with Claude's Task)…
|
||||
assert "Agent" not in dis
|
||||
# …but it is still a read-only conversational role: no shell, no editing.
|
||||
assert "run_terminal_cmd" in dis
|
||||
assert "search_replace" in dis
|
||||
|
||||
|
||||
def test_web_search_disabled_for_every_role() -> None:
|
||||
for role in ("developer", "prompter", "secretary", "main_pm", "pr_reviewer"):
|
||||
assert "--disable-web-search" in gc.grok_cli_args_for_role(role)
|
||||
|
||||
|
||||
def test_effort_is_fleet_override_only(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("ROBOCO_GROK_REASONING_EFFORT", "high")
|
||||
assert (
|
||||
gc.grok_cli_args("be-dev-1")[gc.grok_cli_args("be-dev-1").index("--effort") + 1]
|
||||
== "high"
|
||||
)
|
||||
# "full" disables the per-role reduction entirely.
|
||||
args = gc.grok_cli_args("be-dev-1")
|
||||
assert args[args.index("--effort") + 1] == "high"
|
||||
# "full" / "default" / empty keep grok's model default (no --effort).
|
||||
monkeypatch.setenv("ROBOCO_GROK_REASONING_EFFORT", "full")
|
||||
assert "--effort" not in gc.grok_cli_args("main-pm")
|
||||
monkeypatch.delenv("ROBOCO_GROK_REASONING_EFFORT", raising=False)
|
||||
assert "--effort" not in gc.grok_cli_args("documenter")
|
||||
|
||||
|
||||
def test_max_turns_is_emitted() -> None:
|
||||
args = gc.grok_cli_args("be-dev-1", max_turns=7)
|
||||
assert args[args.index("--max-turns") + 1] == "7"
|
||||
|
||||
|
||||
def test_bash_roles_deny_the_full_git_mutation_set() -> None:
|
||||
# Graceful native --deny rules (the agent recovers) covering the same git
|
||||
# network / branch / history ops the Claude bash-guard blocks.
|
||||
args = gc.grok_cli_args_for_role("developer")
|
||||
for op in ("push", "fetch", "clone", "checkout", "merge", "rebase", "revert"):
|
||||
assert f"Bash(git {op}*)" in args
|
||||
assert "Bash(rm -rf*)" in args
|
||||
|
||||
|
||||
def test_bash_guard_hook_config_skips_git() -> None:
|
||||
handler = gc.bash_guard_hook_config("/app/scripts/bash-guard-hook.sh")[
|
||||
"hooks"
|
||||
]["PreToolUse"][0]
|
||||
assert handler["matcher"] == "Bash"
|
||||
inner = handler["hooks"][0]
|
||||
assert inner["command"] == "/app/scripts/bash-guard-hook.sh"
|
||||
# Git is handled by graceful --deny, so the hook skips it (exfil only).
|
||||
assert inner["env"]["ROBOCO_GUARD_SKIP_GIT"] == "1"
|
||||
|
||||
|
||||
def test_write_grok_hooks_installs_when_script_present(tmp_path: Path) -> None:
|
||||
script = tmp_path / "bash-guard-hook.sh"
|
||||
script.write_text("#!/bin/bash\nexit 0\n", encoding="utf-8")
|
||||
hooks_dir = tmp_path / ".grok" / "hooks"
|
||||
assert gc.write_grok_hooks(hooks_dir=hooks_dir, hook_path=str(script)) is True
|
||||
written = json.loads((hooks_dir / "roboco-bash-guard.json").read_text())
|
||||
assert written["hooks"]["PreToolUse"][0]["matcher"] == "Bash"
|
||||
|
||||
|
||||
def test_write_grok_hooks_noops_when_script_absent(tmp_path: Path) -> None:
|
||||
hooks_dir = tmp_path / ".grok" / "hooks"
|
||||
assert (
|
||||
gc.write_grok_hooks(hooks_dir=hooks_dir, hook_path=str(tmp_path / "nope.sh"))
|
||||
is False
|
||||
)
|
||||
assert not hooks_dir.exists()
|
||||
|
||||
Reference in New Issue
Block a user