grok skill: cygpath path-hardening + review-files/review-diff modes

Fixes the two Windows pain points when routing code review to the Grok CLI (native Windows grok.exe driven from Git Bash): - winpath() (cygpath -w; no-op off Windows) on every path handed to grok.exe (--prompt-file, --cwd) -> deterministic, space-safe; removes reliance on MSYS's argv auto-conversion heuristic (the 'confounded by Windows paths'). - review mode resolves to an absolute Windows path (handles absolute/spaced paths). - NEW review-files [-i instr] <f1> [f2...]: review a set of files together. - NEW review-diff [-C <repo-dir>] [-i instr] <gitref> [-- <pathspec>]: review a git diff; -C targets submodules (e.g. guru-rmm). Diff goes via --prompt-file, not a shell arg -> no 'quote hell'. Tested: text, review (spaced abs path), review-files (2 tray modules), review-diff (self-review of these changes). SKILL.md updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-04 20:45:48 -07:00
parent fdec4b7772
commit 4ab272faab
2 changed files with 91 additions and 8 deletions
--- a/.claude/skills/grok/SKILL.md
+++ b/.claude/skills/grok/SKILL.md
@@ -32,7 +32,9 @@ bash "$CLAUDETOOLS_ROOT/.claude/skills/grok/scripts/ask-grok.sh" <mode> ...
 |------|-------|--------------|
 | `text` | `ask-grok.sh text "<prompt>"` or `text --prompt-file <path>` | One-shot text answer (independent model). `--prompt-file` for long content (review/summarize a doc). |
 | `verify` | `ask-grok.sh verify "<claim/finding>"` or `verify --prompt-file <path>` | Adversarial second opinion — Grok tries to REFUTE/find gaps, returns a verdict + reasons. |
-| `review` | `ask-grok.sh review <file-path> ["<instructions>"]` | Grok reads the file at `<path>` itself (its `read_file` tool, run in the repo) and reviews it — no embedding, handles large files, can pull in referenced files. |
+| `review` | `ask-grok.sh review <file-path> ["<instructions>"]` | Grok reads the file at `<path>` itself (its `read_file` tool) and reviews it — no embedding, handles large files, can pull in referenced files. Accepts absolute or repo-relative paths, and paths with spaces. |
+| `review-files` | `ask-grok.sh review-files [-i "<instr>"] <f1> [f2 …]` | Review a **set** of files together (grok `read_file`s each) — for cross-file consistency or a multi-file change. Paths absolute or repo-relative; spaces OK. No code passed as a shell arg → no quote hell. |
+| `review-diff` | `ask-grok.sh review-diff [-C <repo-dir>] [-i "<instr>"] <gitref> [-- <pathspec>]` | Review a **git diff** (`git diff <gitref>` from `<repo-dir>`; default repo root, use `-C` for a submodule e.g. `-C projects/msp-tools/guru-rmm`). The diff goes via the prompt file (not a shell arg); grok can `read_file` changed files for full context (cwd = repo dir). |
 | `image` | `ask-grok.sh image "<prompt>" [out.png]` | `image_gen` (Imagine) → copies the artifact to `out` (default `grok-image.png`). |
 | `video` | `ask-grok.sh video "<motion prompt>" <input-image> [out.mp4]` | `image_to_video` on an input image → copies to `out`. ~60-90s. |
 | `xsearch` | `ask-grok.sh xsearch "<query>"` | Live `web_search` + X/Twitter tools; returns text with citations. |
--- a/.claude/skills/grok/scripts/ask-grok.sh
+++ b/.claude/skills/grok/scripts/ask-grok.sh
@@ -17,6 +17,9 @@
 #   ask-grok.sh image   "<prompt>" [out.png]               # image_gen -> copy artifact to out
 #   ask-grok.sh video   "<prompt>" <input-image> [out.mp4] # image_to_video on input image
 #   ask-grok.sh xsearch "<query>"                          # live X/Twitter + web search
+#   ask-grok.sh review  <file> [instructions]              # grok read_file's + reviews one file
+#   ask-grok.sh review-files [-i "instr"] <f1> [f2 ...]    # review a SET of files together
+#   ask-grok.sh review-diff [-C <repo-dir>] [-i "instr"] <gitref> [-- <pathspec>]  # review a git diff
 #   ask-grok.sh raw     <grok args...>                     # escape hatch (passes through)
 #
 # Exit: 0 ok, 1 no result/artifact, 2 usage, 127 grok not found.
@@ -26,6 +29,17 @@ SELF="ask-grok"
 PY="$(command -v py 2>/dev/null || command -v python 2>/dev/null || command -v python3 2>/dev/null || true)"
 [ -z "$PY" ] && { echo "[$SELF] python (py/python/python3) required for JSON parsing" >&2; exit 127; }

+# --- path conversion: native-Windows path for grok.exe args (no-op off Windows) ---
+# grok.exe is a native Windows binary; Git Bash hands it POSIX paths (/tmp, /c/.., /d/..)
+# that it cannot resolve. cygpath -w converts to C:\... form on MSYS/Cygwin; on Linux/macOS
+# (native grok, already-correct paths) it passes through unchanged. Doing this explicitly
+# removes reliance on MSYS's heuristic auto-conversion (which breaks on spaces/edge cases).
+if command -v cygpath >/dev/null 2>&1; then
+  winpath() { cygpath -w -- "$1" 2>/dev/null || printf '%s' "$1"; }
+else
+  winpath() { printf '%s' "$1"; }
+fi
+
 # --- identity.json (per-machine, gitignored) declares whether grok is installed here ---
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" 2>/dev/null && pwd)"
 IDFILE=""
@@ -62,7 +76,7 @@ fi
 [ -z "$GROK" ] && { echo "[$SELF] grok CLI not found (set identity.json grok.binary, GROK=, or install grok)" >&2; exit 127; }

 MODE="${1:-}"; shift 2>/dev/null || true
-[ -z "$MODE" ] && { echo "usage: $SELF {text|verify|image|video|xsearch|raw} ..." >&2; exit 2; }
+[ -z "$MODE" ] && { echo "usage: $SELF {text|verify|image|video|xsearch|review|review-files|review-diff|raw} ..." >&2; exit 2; }

 TMP="$(mktemp -d)"; trap 'rm -rf "$TMP"' EXIT
 WORK="$TMP/work"; mkdir -p "$WORK"
@@ -80,8 +94,10 @@ fi

 run_grok() {
  local to="$1"; shift
-  "$TIMEOUT_CMD" "$to" "$GROK" --prompt-file "$PF" --output-format json \
-    --permission-mode dontAsk --no-subagents --no-plan --cwd "$RUN_CWD" "$@" \
+  # Hand grok native-Windows paths (cygpath); MSYS leaves already-Windows paths alone,
+  # so conversion is deterministic and space-safe.
+  "$TIMEOUT_CMD" "$to" "$GROK" --prompt-file "$(winpath "$PF")" --output-format json \
+    --permission-mode dontAsk --no-subagents --no-plan --cwd "$(winpath "$RUN_CWD")" "$@" \
    >"$OUT" 2>"$TMP/err.txt" || true
 }

@@ -156,18 +172,83 @@ case "$MODE" in
    [ -z "${1:-}" ] && { echo "usage: $SELF review <file-path> [instructions]" >&2; exit 2; }
    target="$1"
    instr="${2:-Give an independent, critical review of this file: accuracy, gaps/omissions, and concrete improvements. Be specific.}"
-    # Grok reads the file itself (no embedding) -- run it in the repo so read_file resolves repo-relative paths.
-    [ -f "$target" ] || [ -f "$REPO_ROOT/$target" ] || { echo "[$SELF] file not found: $target" >&2; exit 2; }
+    # Grok reads the file itself (no embedding). Resolve to an absolute path (as given, or
+    # repo-relative), then hand grok the native-Windows ABSOLUTE path so read_file works
+    # regardless of cwd, and tolerates absolute paths and spaces.
+    if   [ -f "$target" ];            then resolved="$target"
+    elif [ -f "$REPO_ROOT/$target" ]; then resolved="$REPO_ROOT/$target"
+    else echo "[$SELF] file not found: $target" >&2; exit 2; fi
+    tgt_win="$(winpath "$resolved")"
    RUN_CWD="$REPO_ROOT"
-    printf 'Use your read_file tool to read the file at this path (relative to your current directory), then do the task and stop. You may also read closely-related files it references if that helps. Do not modify anything.\nPath: %s\n\nTask: %s' "$target" "$instr" > "$PF"
+    printf 'Use your read_file tool to read the file at this absolute path, then do the task and stop. You may also read closely-related files it references if that helps. Do not modify anything.\nPath: %s\n\nTask: %s' "$tgt_win" "$instr" > "$PF"
    run_grok 240 --max-turns 12
    txt="$(jfield text)"
    if [ -n "$txt" ]; then printf '%s\n' "$txt"; else
      echo "[$SELF] no result (session=$(jfield sessionId), stopReason=$(jfield stopReason))" >&2; exit 1; fi
    ;;
+  review-files)
+    # review-files [-i "instructions"] <file> [file ...]
+    # Reviews a SET of files together (grok read_file's each). Paths may be absolute or
+    # repo-relative; spaces are fine. No code is passed as a shell arg -> no quote hell.
+    instr='Independently review these files together as a unit: correctness/bugs, gaps, cross-file consistency, and concrete improvements. Be specific and cite file:line.'
+    files=()
+    while [ $# -gt 0 ]; do
+      case "$1" in
+        -i|--instr) instr="${2:-}"; shift 2 2>/dev/null || shift ;;
+        *)          files+=("$1"); shift ;;
+      esac
+    done
+    [ ${#files[@]} -eq 0 ] && { echo "usage: $SELF review-files [-i \"instructions\"] <file> [file ...]" >&2; exit 2; }
+    list=""
+    for f in "${files[@]}"; do
+      if   [ -f "$f" ];            then r="$f"
+      elif [ -f "$REPO_ROOT/$f" ]; then r="$REPO_ROOT/$f"
+      else echo "[$SELF] file not found: $f" >&2; exit 2; fi
+      list+="- $(winpath "$r")
+"
+    done
+    RUN_CWD="$REPO_ROOT"
+    printf 'Use your read_file tool to read EACH of these files (absolute paths), then perform the task across ALL of them and stop. Do not modify anything.\n\nFiles:\n%s\nTask: %s' "$list" "$instr" > "$PF"
+    run_grok 300 --max-turns 24
+    txt="$(jfield text)"
+    if [ -n "$txt" ]; then printf '%s\n' "$txt"; else
+      echo "[$SELF] no result (session=$(jfield sessionId), stopReason=$(jfield stopReason))" >&2; exit 1; fi
+    ;;
+  review-diff)
+    # review-diff [-C <repo-dir>] [-i "instructions"] <gitref> [-- <pathspec...>]
+    # Reviews `git diff <gitref>` from <repo-dir> (default repo root; use -C for a submodule,
+    # e.g. -C projects/msp-tools/guru-rmm). The diff is written to the prompt file (not a shell
+    # arg) -> no quote hell; grok can read_file changed files for full context (cwd=repo-dir).
+    gdir="$REPO_ROOT"
+    instr='Review this git diff: correctness/bugs introduced, regressions, missing edge cases, and concrete fixes. Focus on the CHANGES. Be specific and cite file:line.'
+    ref=""; pathspec=()
+    while [ $# -gt 0 ]; do
+      case "$1" in
+        -C|--dir)   gdir="${2:-}"; shift 2 2>/dev/null || shift ;;
+        -i|--instr) instr="${2:-}"; shift 2 2>/dev/null || shift ;;
+        --)         shift; while [ $# -gt 0 ]; do pathspec+=("$1"); shift; done ;;
+        *)          if [ -z "$ref" ]; then ref="$1"; else pathspec+=("$1"); fi; shift ;;
+      esac
+    done
+    [ -z "$ref" ] && { echo "usage: $SELF review-diff [-C <repo-dir>] [-i \"instr\"] <gitref> [-- <pathspec>]" >&2; exit 2; }
+    [ -d "$gdir" ] || { [ -d "$REPO_ROOT/$gdir" ] && gdir="$REPO_ROOT/$gdir"; }
+    git -C "$gdir" rev-parse --git-dir >/dev/null 2>&1 || { echo "[$SELF] not a git repo: $gdir" >&2; exit 2; }
+    if [ ${#pathspec[@]} -gt 0 ]; then
+      git -C "$gdir" diff "$ref" -- "${pathspec[@]}" > "$TMP/diff.txt" 2>"$TMP/differr.txt"
+    else
+      git -C "$gdir" diff "$ref" > "$TMP/diff.txt" 2>"$TMP/differr.txt"
+    fi
+    [ -s "$TMP/diff.txt" ] || { echo "[$SELF] empty/failed diff for '$ref' in $gdir: $(head -1 "$TMP/differr.txt" 2>/dev/null)" >&2; exit 1; }
+    RUN_CWD="$gdir"   # changed-file paths in the diff are relative to this repo root
+    { printf 'Review the following unified git diff. %s\nYou may use read_file on any changed file (paths in the diff are relative to your current directory; strip the a/ b/ prefixes) for full context. Do not modify anything.\n\n=== BEGIN DIFF ===\n' "$instr"; cat "$TMP/diff.txt"; printf '\n=== END DIFF ===\n'; } > "$PF"
+    run_grok 300 --max-turns 20
+    txt="$(jfield text)"
+    if [ -n "$txt" ]; then printf '%s\n' "$txt"; else
+      echo "[$SELF] no result (session=$(jfield sessionId), stopReason=$(jfield stopReason))" >&2; exit 1; fi
+    ;;
  raw)
    "$GROK" "$@"
    ;;
  *)
-    echo "[$SELF] unknown mode '$MODE' (use text|verify|image|video|xsearch|raw)" >&2; exit 2 ;;
+    echo "[$SELF] unknown mode '$MODE' (use text|verify|image|video|xsearch|review|review-files|review-diff|raw)" >&2; exit 2 ;;
 esac