From 4ab272faabe63fc95eae49ad5adfe4d21c964cd7 Mon Sep 17 00:00:00 2001 From: Mike Swanson Date: Thu, 4 Jun 2026 20:45:48 -0700 Subject: [PATCH] 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] [f2...]: review a set of files together. - NEW review-diff [-C ] [-i instr] [-- ]: 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) --- .claude/skills/grok/SKILL.md | 4 +- .claude/skills/grok/scripts/ask-grok.sh | 95 +++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/.claude/skills/grok/SKILL.md b/.claude/skills/grok/SKILL.md index 27ec3eb..35138bf 100644 --- 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" ... |------|-------|--------------| | `text` | `ask-grok.sh text ""` or `text --prompt-file ` | One-shot text answer (independent model). `--prompt-file` for long content (review/summarize a doc). | | `verify` | `ask-grok.sh verify ""` or `verify --prompt-file ` | Adversarial second opinion — Grok tries to REFUTE/find gaps, returns a verdict + reasons. | -| `review` | `ask-grok.sh review [""]` | Grok reads the file at `` 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 [""]` | Grok reads the file at `` 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 ""] [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 ] [-i ""] [-- ]` | Review a **git diff** (`git diff ` from ``; 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 "" [out.png]` | `image_gen` (Imagine) → copies the artifact to `out` (default `grok-image.png`). | | `video` | `ask-grok.sh video "" [out.mp4]` | `image_to_video` on an input image → copies to `out`. ~60-90s. | | `xsearch` | `ask-grok.sh xsearch ""` | Live `web_search` + X/Twitter tools; returns text with citations. | diff --git a/.claude/skills/grok/scripts/ask-grok.sh b/.claude/skills/grok/scripts/ask-grok.sh index 70144aa..3143f86 100644 --- a/.claude/skills/grok/scripts/ask-grok.sh +++ b/.claude/skills/grok/scripts/ask-grok.sh @@ -17,6 +17,9 @@ # ask-grok.sh image "" [out.png] # image_gen -> copy artifact to out # ask-grok.sh video "" [out.mp4] # image_to_video on input image # ask-grok.sh xsearch "" # live X/Twitter + web search +# ask-grok.sh review [instructions] # grok read_file's + reviews one file +# ask-grok.sh review-files [-i "instr"] [f2 ...] # review a SET of files together +# ask-grok.sh review-diff [-C ] [-i "instr"] [-- ] # review a git diff # ask-grok.sh raw # 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 [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 ...] + # 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 ...]" >&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 ] [-i "instructions"] [-- ] + # Reviews `git diff ` from (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 ] [-i \"instr\"] [-- ]" >&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