feat(autoreview): add grok review engine

modules/ai/skills/autoreview/SKILL.md

@@ -7,11 +7,11 @@ description: "Auto Review closeout for Git and Jujutsu changes. Uses Amp by defa
 
 Run the bundled structured review helper as a closeout check. This is code review, not Guardian `auto_review` approval routing.
 
-Amp is the default inside an Amp orb, detected by the documented `AMP_ORB=1` environment variable. Codex is the default elsewhere and usually delivers the best local review results. An explicit `--engine` always wins; `AUTOREVIEW_ENGINE` overrides the environment-based default. Codex defaults to `gpt-5.6-sol` and retries once with `gpt-5.6-terra` only when the account cannot access Sol; thinking follows the Codex CLI config. Claude defaults to `claude-fable-5`. Amp defaults to `openai/gpt-5.6-sol` at `high` reasoning through a generated adapter plugin that reuses the existing `amp login`. Pi and opencode use the model their own CLI is configured for.
+Amp is the default inside an Amp orb, detected by the documented `AMP_ORB=1` environment variable. Codex is the default elsewhere and usually delivers the best local review results. An explicit `--engine` always wins; `AUTOREVIEW_ENGINE` overrides the environment-based default. Codex defaults to `gpt-5.6-sol` and retries once with `gpt-5.6-terra` only when the account cannot access Sol; thinking follows the Codex CLI config. Claude defaults to `fable`, the claude CLI alias for the latest Fable. Amp defaults to `openai/gpt-5.6-sol` at `high` reasoning through a generated adapter plugin that reuses the existing `amp login`. Grok defaults to `grok-4.6`. Pi and opencode use the model their own CLI is configured for.
 
 Use when:
 
-- user asks for Codex review / Claude review / Amp review / Pi review / autoreview / second-model review
+- user asks for Codex review / Claude review / Amp review / Grok review / Pi review / autoreview / second-model review
 - after non-trivial code edits, before final/commit/ship
 - reviewing a local branch or PR branch after fixes
 
@@ -175,14 +175,22 @@ Inline syntax is also supported:
 <autoreview-helper> --reviewers codex:gpt-5.6-sol:high,claude:sonnet:max
 ```
 
+The same engine may appear more than once with different models, which is how
+an Amp orb runs the codex+grok panel through amp's model providers:
+
+```bash
+<autoreview-helper> --reviewers amp:openai/gpt-5.6-sol:xhigh,amp:xai/grok-4.6:xhigh
+```
+
 `AUTOREVIEW_MODEL` and `AUTOREVIEW_THINKING` env vars accept the same keyed
 syntax (`codex=gpt-5.5,claude=sonnet` or a bare global value) and sit between
 CLI flags and built-in defaults.
 
 Thinking per engine: Codex maps to `model_reasoning_effort` (`low`-`max`).
 Claude maps to `--effort` (`low`-`max`). Amp maps to the adapter plugin's
 `reasoningEffort` (`none`-`max`, default `high`) and its model must be a
-`provider/model` id (default `openai/gpt-5.6-sol`). Pi maps to `--thinking`
+`provider/model` id (default `openai/gpt-5.6-sol`). Grok maps to `--effort`
+(`low`-`xhigh`). Pi maps to `--thinking`
 (`off`-`max`). OpenCode maps to `--variant` (`minimal`-`max`). Engines
 without a real thinking knob reject `--thinking`.
 
@@ -215,15 +223,15 @@ The helper:
   fetch errors; use `--no-fetch` only when intentionally reviewing local refs
 - exits successfully without invoking any engine when the computed diff has no
   changed paths
-- supports `--engine codex`, `claude`, `amp`, `droid`, `copilot`, `pi`, and `opencode`; default precedence is explicit `--engine`, `AUTOREVIEW_ENGINE`, Amp when `AMP_ORB=1`, then Codex
+- supports `--engine codex`, `claude`, `amp`, `grok`, `droid`, `copilot`, `pi`, and `opencode`; default precedence is explicit `--engine`, `AUTOREVIEW_ENGINE`, Amp when `AMP_ORB=1`, then Codex
 - use `--mode commit --commit <ref>` for already-committed work, especially clean `main` after landing
 - should be left in `--mode auto` or forced to `--mode branch` for PR/branch work; do not force `--mode local` after committing
 - writes only to stdout unless `--output` or `--json-output` is set
 - reports only findings at or above `--max-priority` / `AUTOREVIEW_MAX_PRIORITY` (default `P2`; upstream defaults to `P0`); lower priorities are omitted from output and exit status
 - supports `--dry-run` as a real preflight: builds the bundle, applies the same input validation as a live run, resolves each reviewer binary, and exits nonzero on a broken setup without invoking any engine
 - supports `--parallel-tests`, `--prompt`, `--prompt-file`, `--dataset`, `--no-tools`, `--no-web-search`, and commit refs
 - supports opt-in review panels with `--panel` / `--reviewers`, plus per-engine `--model` and `--thinking` (also via `AUTOREVIEW_MODEL` / `AUTOREVIEW_THINKING`)
-- allows read-only tools and web search by default where the selected CLI supports them; forbids nested review in the prompt; Codex is run through `codex exec` with read-only sandbox and structured output; amp reviews the bundle alone through a generated `amp.ai.generate` adapter plugin (temp config dir, so no personal plugins load; existing `amp login` credentials are reused); pi gets only its read tool; opencode runs its read-only plan agent
+- allows read-only tools and web search by default where the selected CLI supports them; forbids nested review in the prompt; Codex is run through `codex exec` with read-only sandbox and structured output; amp reviews the bundle alone through a generated `amp.ai.generate` adapter plugin (temp config dir, so no personal plugins load; existing `amp login` credentials are reused); grok runs headless with only `read_file`/`grep`/`list_dir` (plus web tools), subagents and MCP meta-tools disabled, prompt via file, session deleted afterwards (grok has no ephemeral mode), and rejects `--no-tools` (grok has no reliable tool-off switch); like claude and codex it runs inside the reviewed checkout under the local trust model, so project hooks/config there apply; pi gets only its read tool; opencode runs its read-only plan agent
 - keeps droid and copilot adapters even though upstream disabled them; they review with the local trust model, not upstream's isolation contract
 - prints `review still running: <engine> elapsed=<seconds>s pid=<pid>` to stderr at long-running intervals while waiting for the selected review engine; `--stream-engine-output` streams live engine text instead
 - prints `autoreview clean: no accepted/actionable findings reported` when the selected review command exits 0

modules/ai/skills/autoreview/UPSTREAM.txt

@@ -9,4 +9,5 @@ upstream_synced_commit = 7c49348abc51e820a344868254ef0f5c35443720
 sync_policy = diverged fork; sync concepts, not code. Do not attempt a file-level merge from upstream; port features selectively and keep the script lean.
 local_changes = adjusted helper paths; removed cache-repair guidance; reject out-of-scope findings (upstream ignores them); native jj local/branch/commit bundles and non-colocated harness coverage; --head/--remote/--no-fetch; empty-diff early exit; 5-round cap + nitpick escape hatch in SKILL.md
 synced_from_upstream_2026_08 = engines amp/pi/opencode (amp uses upstream's generated amp.ai.generate adapter plugin but reuses the existing amp login via real XDG data dir + temp config dir, no plugin-inventory attestation; pi read-tool only; opencode plan agent); model defaults gpt-5.6-sol/claude-fable-5 + Codex Sol-to-Terra account-access fallback; AUTOREVIEW_MODEL/AUTOREVIEW_THINKING env defaults; P0-P3 --max-priority filter (default P2 here; upstream defaults P0) + threshold prompt; real --dry-run preflight; --stream-engine-output with filtered Codex/Claude displays; SKILL.md scope governor + P0 contract + prose-only exemption
+local_engines = grok (headless `grok --prompt-file --output-format json --json-schema`, read-only tool allowlist, no subagents; not in upstream)
 intentionally_not_synced = TruffleHog secret scanning + redaction pipeline; reviewer workspace/env isolation and process-group machinery; kimi + cursor engines; upstream's refusal of droid/copilot/opencode (kept working under local trust model); resumable --run-id/--run-root chunked passes; Windows harnesses; hardening test suite

modules/ai/skills/autoreview/scripts/autoreview

@@ -14,17 +14,19 @@ import tempfile
 import textwrap
 import threading
 import time
+import uuid
 from pathlib import Path
 from typing import Any
 from urllib.parse import urlparse
 
 
-ENGINES = ("codex", "claude", "amp", "droid", "copilot", "pi", "opencode")
+ENGINES = ("codex", "claude", "amp", "grok", "droid", "copilot", "pi", "opencode")
 THINKING_LEVELS_BY_ENGINE = {
     "codex": {"low", "medium", "high", "xhigh", "max"},
     "claude": {"low", "medium", "high", "xhigh", "max"},
     # amp maps thinking to amp.ai.generate reasoningEffort in the adapter plugin
     "amp": {"none", "low", "medium", "high", "xhigh", "max"},
+    "grok": {"low", "medium", "high", "xhigh"},
     "droid": set(),
     "copilot": set(),
     "pi": {"off", "minimal", "low", "medium", "high", "xhigh", "max"},
@@ -33,8 +35,9 @@ THINKING_LEVELS_BY_ENGINE = {
 }
 DEFAULT_MODEL_BY_ENGINE = {
     "codex": "gpt-5.6-sol",
-    "claude": "claude-fable-5",
+    "claude": "fable",  # claude CLI alias for the latest Fable
     "amp": "openai/gpt-5.6-sol",
+    "grok": "grok-4.6",
 }
 DEFAULT_THINKING_BY_ENGINE = {
     "amp": "high",
@@ -49,6 +52,17 @@ AMP_ADAPTER_MODE = "autoreview"
 AMP_ADAPTER_AGENT = "autoreview-adapter"
 AMP_OUTER_MODEL = "openai/gpt-5.6-luna"
 AMP_MAX_OUTPUT_CHARS = 2_000_000
+# grok's built-in tool names; review stays read-only.
+GROK_READ_TOOLS = ("read_file", "grep", "list_dir")
+GROK_WEB_TOOLS = ("web_search", "web_fetch")
+# --tools only allowlists built-ins; the MCP meta-tools stay unless denied.
+GROK_DENY_TOOLS = ("search_tool", "use_tool")
+# Engines with no reliable tool-off switch; validated before dry-run so preflight matches live runs.
+NO_TOOLS_UNSUPPORTED = {
+    "codex": "--no-tools is not supported by the Codex engine; use --engine claude --no-tools for a no-tools run",
+    "grok": "--no-tools is not supported by the grok engine; use --engine claude --no-tools for a no-tools run",
+    "copilot": "--no-tools is not supported by the copilot engine; copilot requires a read-only file view tool to load the review bundle without exposing it in argv",
+}
 AMP_MODEL_PATTERN = re.compile(
     r"(?:amp|anthropic|baseten|fireworks|openai|vertexai|xai)/"
     r"[A-Za-z0-9][A-Za-z0-9._-]*(?:/[A-Za-z0-9][A-Za-z0-9._-]*)*"
@@ -715,9 +729,6 @@ def codex_model_access_failure(result: subprocess.CompletedProcess[str], model:
 
 
 def run_codex(args: argparse.Namespace, repo: Path, prompt: str) -> str:
-    if not args.tools:
-        raise SystemExit("--no-tools is not supported by the Codex engine; use --engine claude --no-tools for a no-tools run")
-
     def attempt(model: str | None) -> tuple[subprocess.CompletedProcess[str], str]:
         schema_path = write_json_temp(SCHEMA)
         output_path = Path(tempfile.NamedTemporaryFile("w", suffix=".json", delete=False).name)
@@ -812,6 +823,70 @@ def run_claude(args: argparse.Namespace, repo: Path, prompt: str) -> str:
     return result.stdout
 
 
+def run_grok(args: argparse.Namespace, repo: Path, prompt: str) -> str:
+    # An empty or unknown --tools allowlist falls back to grok's default toolset and
+    # plan mode still allows writes, so the allowlist below is the only read-only gate.
+    # Bundles can exceed ARG_MAX, so the prompt goes through a file.
+    prompt_path = Path(tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False).name)
+    prompt_path.write_text(prompt)
+    os.chmod(prompt_path, 0o600)
+    # grok has no ephemeral mode; a preassigned id lets cleanup delete the session even on failure.
+    session_id = str(uuid.uuid4())
+    cmd = [
+        args.grok_bin,
+        "--session-id",
+        session_id,
+        "--prompt-file",
+        str(prompt_path),
+        "--output-format",
+        "json",
+        "--json-schema",
+        json.dumps(SCHEMA),
+        "--cwd",
+        str(repo),
+        "--no-subagents",
+        "--disallowed-tools",
+        ",".join(GROK_DENY_TOOLS),
+    ]
+    tools = list(GROK_READ_TOOLS)
+    if args.web_search:
+        tools.extend(GROK_WEB_TOOLS)
+    cmd.extend(["--tools", ",".join(tools)])
+    if not args.web_search:
+        cmd.append("--disable-web-search")
+    if args.model:
+        cmd.extend(["--model", args.model])
+    if args.thinking:
+        cmd.extend(["--effort", args.thinking])
+    # grok's json output arrives only at exit, so streaming is passthrough of stderr chatter.
+    result = None
+    try:
+        result = run_with_heartbeat(
+            cmd,
+            repo,
+            label="grok",
+            stream_display=passthrough_stream_display if args.stream_engine_output else None,
+        )
+    finally:
+        prompt_path.unlink(missing_ok=True)
+        # On a failed run the session may not exist; report but keep the engine error primary.
+        delete_grok_session(args.grok_bin, session_id, repo, strict=result is not None and result.returncode == 0)
+    if result.returncode != 0:
+        raise SystemExit(f"grok engine failed ({result.returncode})\n{result.stderr or result.stdout}")
+    return result.stdout
+
+
+def delete_grok_session(grok_bin: str, session_id: str, repo: Path, *, strict: bool) -> None:
+    """Drop the headless session so the review bundle does not persist under ~/.grok."""
+    deleted = subprocess.run([grok_bin, "sessions", "delete", session_id], cwd=repo, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    if deleted.returncode == 0:
+        return
+    message = f"grok: failed to delete session {session_id}: {deleted.stderr.strip() or deleted.stdout.strip()}"
+    if strict:
+        raise SystemExit(message)
+    print(message, file=sys.stderr)
+
+
 def amp_review_plugin_source(
     prompt_path: Path,
     result_path: Path,
@@ -1047,8 +1122,6 @@ def run_droid(args: argparse.Namespace, repo: Path, prompt: str) -> str:
 def run_copilot(args: argparse.Namespace, repo: Path, prompt: str) -> str:
     if args.thinking:
         raise SystemExit("--thinking is not supported by the copilot engine")
-    if not args.tools:
-        raise SystemExit("--no-tools is not supported by the copilot engine; copilot requires a read-only file view tool to load the review bundle without exposing it in argv")
     with tempfile.TemporaryDirectory(prefix="autoreview-copilot.") as tempdir:
         prompt_path = Path(tempdir) / "prompt.txt"
         prompt_path.write_text(prompt)
@@ -1110,6 +1183,12 @@ def extract_json(text: str) -> dict[str, Any]:
         return parsed
     if isinstance(parsed, dict) and isinstance(parsed.get("structured_output"), dict):
         return parsed["structured_output"]
+    if isinstance(parsed, dict) and isinstance(parsed.get("structuredOutput"), dict):
+        # grok: a max-turns/cancelled run still carries the last provisional object.
+        stop_reason = parsed.get("stopReason")
+        if stop_reason not in (None, "end_turn"):
+            raise SystemExit(f"review engine stopped early ({stop_reason}); refusing partial structured output")
+        return parsed["structuredOutput"]
     if isinstance(parsed, dict) and isinstance(parsed.get("result"), dict) and "findings" in parsed["result"]:
         return parsed["result"]
     if isinstance(parsed, dict) and isinstance(parsed.get("result"), str):
@@ -1371,7 +1450,7 @@ def parse_args() -> argparse.Namespace:
         action="append",
         help=(
             "Thinking/effort for all reviewers or engine=level. Repeatable. Overrides AUTOREVIEW_THINKING. "
-            "Codex/Claude: low, medium, high, xhigh, max. Amp (agent mode): low, medium, high, ultra. "
+            "Codex/Claude: low, medium, high, xhigh, max. Amp (agent mode): low, medium, high, ultra. Grok: low, medium, high, xhigh. "
             "Pi: off, minimal, low, medium, high, xhigh, max. OpenCode: minimal, low, medium, high, max."
         ),
     )
@@ -1385,6 +1464,7 @@ def parse_args() -> argparse.Namespace:
     parser.add_argument("--codex-bin", default=os.environ.get("CODEX_BIN", "codex"))
     parser.add_argument("--claude-bin", default=os.environ.get("CLAUDE_BIN", "claude"))
     parser.add_argument("--amp-bin", default=os.environ.get("AMP_BIN", "amp"))
+    parser.add_argument("--grok-bin", default=os.environ.get("GROK_BIN", "grok"))
     parser.add_argument("--droid-bin", default=os.environ.get("DROID_BIN", "droid"))
     parser.add_argument("--copilot-bin", default=os.environ.get("COPILOT_BIN", "copilot"))
     parser.add_argument("--pi-bin", default=os.environ.get("PI_BIN", "pi"))
@@ -1395,7 +1475,7 @@ def parse_args() -> argparse.Namespace:
         default=os.environ.get("AUTOREVIEW_STREAM_ENGINE_OUTPUT") == "1",
         help="Stream engine output while preserving buffered output for validation. Codex and Claude filter noisy tool/status chatter.",
     )
-    parser.add_argument("--no-tools", dest="tools", action="store_false", default=True, help="Disable tools for engines that support it. Codex and copilot reject no-tools review. Amp always reviews the bundle alone; pi gets read-only tools.")
+    parser.add_argument("--no-tools", dest="tools", action="store_false", default=True, help="Disable tools for engines that support it. Codex, grok, and copilot reject no-tools review. Amp always reviews the bundle alone; pi gets read-only tools.")
     parser.add_argument("--no-web-search", dest="web_search", action="store_false", default=True)
     parser.add_argument(
         "--claude-allowed-tools",
@@ -1428,6 +1508,8 @@ def run_engine(args: argparse.Namespace, repo: Path, prompt: str) -> str:
         return run_claude(args, repo, prompt)
     if args.engine == "amp":
         return run_amp(args, repo, prompt)
+    if args.engine == "grok":
+        return run_grok(args, repo, prompt)
     if args.engine == "droid":
         return run_droid(args, repo, prompt)
     if args.engine == "copilot":
@@ -1503,12 +1585,9 @@ def reviewer_args(args: argparse.Namespace) -> list[argparse.Namespace]:
     else:
         reviewers = [(args.engine, None, None)]
 
-    seen: set[str] = set()
+    seen: set[tuple[str, str | None]] = set()
     result: list[argparse.Namespace] = []
     for engine, inline_model, inline_thinking in reviewers:
-        if engine in seen:
-            raise SystemExit(f"reviewer specified more than once: {engine}")
-        seen.add(engine)
         model = (
             inline_model
             or model_by_engine.get(engine)
@@ -1525,6 +1604,10 @@ def reviewer_args(args: argparse.Namespace) -> list[argparse.Namespace]:
             or env_global_thinking
             or DEFAULT_THINKING_BY_ENGINE.get(engine)
         )
+        # The same engine may appear with different models (amp panels: sol + grok).
+        if (engine, model) in seen:
+            raise SystemExit(f"reviewer specified more than once: {engine} model={model}")
+        seen.add((engine, model))
         if engine == "codex" and model == DEFAULT_MODEL_BY_ENGINE["codex"]:
             fallback_model = DEFAULT_CODEX_ACCESS_FALLBACK_MODEL
         else:
@@ -1621,13 +1704,21 @@ ENGINE_BIN_ATTR = {
     "codex": "codex_bin",
     "claude": "claude_bin",
     "amp": "amp_bin",
+    "grok": "grok_bin",
     "droid": "droid_bin",
     "copilot": "copilot_bin",
     "pi": "pi_bin",
     "opencode": "opencode_bin",
 }
 
 
+def validate_engine_options(args: argparse.Namespace, reviewers: list[argparse.Namespace]) -> None:
+    """Engine-runtime checks shared by dry-run and live runs; after the empty-diff guard."""
+    for reviewer in reviewers:
+        if not args.tools and reviewer.engine in NO_TOOLS_UNSUPPORTED:
+            raise SystemExit(NO_TOOLS_UNSUPPORTED[reviewer.engine])
+
+
 def dry_run_preflight(reviewers: list[argparse.Namespace]) -> int:
     """Validate everything a real run needs except the engine calls: the
     bundle/prompt built above and each reviewer's CLI binary."""
@@ -1693,6 +1784,7 @@ def main() -> int:
     if not changed_paths:
         print("empty diff: nothing to review")
         return 0
+    validate_engine_options(args, reviewers)
     if args.dry_run:
         return dry_run_preflight(reviewers)