OPS: Harden Windows Git resolution and index.lock reliability

[Chris0Jeky]

Summary

• Add scripts/check-git-env.sh diagnostic script that validates git resolves to Git for Windows (not Cygwin) and detects stale .git/index.lock files with no active git process
• Update CLAUDE.md and AGENTS.md Windows sections to reference the new script and add PATH remediation guidance

Closes #121

Validation

• Script tested locally: confirms non-Cygwin git resolution and clean index.lock state
• Outputs actionable guidance when issues are detected (Cygwin path, stale lock with/without active processes)

Test plan

• Run bash scripts/check-git-env.sh on a Windows machine with Git for Windows — should pass
• Simulate Cygwin git by temporarily aliasing — script should warn
• Create a fake .git/index.lock with no git processes — script should detect and advise removal
• Verify CLAUDE.md and AGENTS.md changes are accurate and consistent

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Self-Review

Findings

Bug found and fixed (commit 8ca8896):

• index.lock detection originally used $REPO_DIR/.git/index.lock, which fails in git worktrees where .git is a file, not a directory. Fixed to use git rev-parse --git-dir to resolve the actual git directory.

Remaining observations (no action needed)

• Cygwin detection heuristic: The path-based check (/cygdrive/* and /usr/bin/git) covers the common Cygwin layouts. An exotic Cygwin install with a non-standard prefix could slip through, but the version-string check (grep -qi "cygwin") provides a second layer of defense. Acceptable coverage.
• tasklist filter syntax: Uses //FI (double slash) which is the correct syntax for Git Bash on Windows (translates to /FI for the native command). Verified working.
• pgrep fallback: The pgrep -a git pattern could match non-git processes with "git" in their name (e.g., "digital"). This is a minor false-positive risk that results in a conservative "lock may be legitimate" message rather than suggesting removal, so it errs on the safe side.
• No destructive actions: Script is read-only/diagnostic. It never removes files or modifies state.

Acceptance criteria coverage

• Shell guardrail that warns on wrong git executable
• Shell guardrail that warns on stale index.lock
• Fallback guidance in AGENTS.md
• Reference in CLAUDE.md
• Validation steps documented (script itself + PR test plan)

[Chris0Jeky]

Fresh Adversarial Review

Critical Issues

1. GIT_DIR relative path bug breaks lock-file detection when using explicit repo argument

git -C "$REPO_DIR" rev-parse --git-dir returns a relative path (.git) even when $REPO_DIR is absolute. The script then checks -f "$GIT_DIR/index.lock", which resolves relative to the current working directory, not relative to $REPO_DIR.

Reproduction:

cd /tmp
bash /path/to/scripts/check-git-env.sh /path/to/repo
# Lock check looks at /tmp/.git/index.lock instead of /path/to/repo/.git/index.lock

Fix: Prepend $REPO_DIR/ when the result is relative, or use git -C "$REPO_DIR" rev-parse --absolute-git-dir (available since Git 2.13).

Minor Issues

2. File mode should be 755, not 644

The script is 100644 but is a shell script intended for direct execution. While bash scripts/check-git-env.sh works regardless, the conventional mode for shell scripts is 100755. This matters if someone tries ./scripts/check-git-env.sh.

3. Comment says "Cygwin" but /usr/bin/git also catches MSYS2's own git package

The case pattern /usr/bin/git will also match MSYS2's non-MinGW git (installed via pacman -S git in MSYS2 shell, as opposed to the MinGW git at /mingw64/bin/git). The comment should note this is detecting "Cygwin or MSYS2 (non-MinGW)" git, since the practical guidance is the same but the diagnostic message would otherwise be confusing to someone who knows they don't have Cygwin installed.

Observations

• CI is fully green. All 13 required checks pass.
• Self-review was thorough. The worktree fix (commit 8ca8896) was a good catch, though it introduced the relative-path bug described above.
• Script is read-only/diagnostic. No destructive actions, which is good.
• set -euo pipefail with accumulated EXIT_CODE: The error handling pattern works because all potentially-failing commands are wrapped with || true. This is correct but slightly unusual -- future editors should be careful not to add bare commands that could fail.
• pgrep -a git false positives: Already noted in self-review. The conservative behavior (reporting lock as potentially legitimate) is the right call.
• tasklist //FI double-slash: Correct for Git Bash's MSYS path translation. Would break on native bash (e.g., Linux), but tasklist wouldn't exist there anyway so the pgrep fallback handles it.
• Acceptance criteria: All five criteria from issue OPS: Harden Windows Git resolution and index.lock reliability for local/agent workflows #121 are addressed. The first criterion ("In a fresh PowerShell session, git resolves to Git for Windows first") is partially met -- the script detects the problem but doesn't enforce resolution. The issue text says "enforce" but the implementation chose "detect and advise," which is arguably more appropriate for a diagnostic script. This seems intentional.

Verdict

One real bug to fix (the relative GIT_DIR path), one minor convention fix (file mode), and one comment accuracy tweak. Otherwise solid work -- the script is well-structured, handles edge cases thoughtfully, and the doc updates are consistent. Fix the critical issue and this is ready to merge.