Skip to content

fix(#5519): code-review --files no longer skips every file on Windows Git Bash - #5643

Merged
trek-e merged 7 commits into
nextfrom
fix/5519-code-review-files-skips-every-file-on-wi
Oct 23, 2026
Merged

trek-e merged 7 commits into
nextfrom
fix/5519-code-review-files-skips-every-file-on-wi

Conversation

@trek-e

@trek-e trek-e commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

Fix PR

Linked Issue

Fixes #5519

What was broken

/gsd-code-review --files reported every file as outside the repository on Windows Git Bash and ended with an empty scope. The Tier 1 containment guard compared git rev-parse --show-toplevel (mixed form, C:/Users/...) against realpath -m output (MSYS POSIX form, /c/Users/...) with a bare "$REPO_ROOT"* prefix test, so no path could ever match. The bare prefix also had no path boundary, so a sibling directory sharing the repository's name prefix was accepted on every platform.

What this fix does

gsd-core/workflows/code-review.md Tier 1 now normalizes both sides through cygpath -m when it exists (a no-op elsewhere), and tests equality-or-root/ boundary instead of a bare prefix. It still fails closed: an empty root, an unresolved .., or a missing realpath (macOS) reports the entry as outside. <platform_notes> documents the cygpath behaviour.

Root cause

Two path forms from two tools compared as strings, plus a prefix test with no boundary. Introduced with the original command (#1630) and recorded as a known unfixed limitation in the #4460 test docblock.

Testing

How I verified the fix

tests/code-review-tier3-files-override-scoping.test.cjs gains a describe "#5519: Tier 1 --files containment guard" that extracts and runs the real fence from code-review.md in bash with stubbed git/realpath/cygpath emitting the Git Bash forms (relative, POSIX-absolute and mixed-absolute accepted; escapes and siblings rejected), a real-bash sibling/escape case, and the fail-closed cases. RED on the test-only commit failed 4 tests (run on 333dfbb81d). GREEN: 54264 tests, 0 failed on 3b8bfe9f59.

Regression test added?

  • Yes

Platforms tested

  • Linux (gsd-test, linux-node24, head 3b8bfe9, passed)

Runtimes tested

  • N/A (not runtime-specific)

Self-test report

{
  "schema": 1,
  "head_sha": "3b8bfe9f593951b9b7003257ed0b49136ebd6ce4",
  "tool": "gsd-test",
  "failing_first": { "test": "tests/code-review-tier3-files-override-scoping.test.cjs (#5519 describe)", "fails_on_base": true, "passes_on_head": true },
  "commands": ["gsd-test --base next --head 3b8bfe9f593951b9b7003257ed0b49136ebd6ce4"],
  "full_suite": { "command": "gsd-test", "result": "passed" },
  "environment": { "os": "darwin driver, linux-node24 bench", "node": "24" },
  "output_excerpt": "verdict passed on linux-node24, head 3b8bfe9f59"
}

Notes for the reviewer

  • One separate commit carries the 3-line package-lock.json change (ip-address 10.7.2 to 10.7.4, GHSA-rxp7-6hxx-x8q5): tests/npm-integrity-gate.test.cjs fails every bench run while the old version is locked. The same hunk is in other open PRs; identical hunks merge cleanly.
  • The real-bash cases skip when GNU realpath is missing; the Git Bash forms are exercised through stubs, so a live Windows run is still the final confirmation.

Checklist

  • Issue linked above with Fixes #NNN
  • Linked issue has the confirmed-bug label
  • Fix is scoped to the reported bug (lockfile commit called out above)
  • Regression test added
  • All existing tests pass
  • .changeset/ fragment added
  • No unnecessary dependencies added

Breaking changes

None.

🤖 Generated with Claude Code

sim and others added 5 commits October 22, 2026 21:03
Lockfile-only. ip-address <= 10.7.3 (transitive, via claude-agent-sdk > @modelcontextprotocol/sdk > express-rate-limit) has a moderate advisory published 2026-10-22; npm audit --omit=dev now reports 0 vulnerabilities.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…every path form and rejects siblings

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…h form and compare on a boundary

Emitted-Drift-Ack-Growth: code-review.md — the Tier 1 containment fence gains a small cygpath normalization helper and a path-boundary comparison (and a platform note) so Windows Git Bash stops rejecting every --files entry.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…latform note to both path forms

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…losed

Emitted-Drift-Ack-Growth: code-review.md — the Tier 1 containment fence gains a small cygpath normalization helper, a path-boundary comparison and two fail-closed guards (and a platform note) so Windows Git Bash stops rejecting every --files entry.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@trek-e
trek-e added this pull request to the merge queue Oct 23, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 23, 2026
… print the compared path forms on failure

The merge-queue Windows shard rejected the in-repository file because the
runner's 8.3 temp path (RUNNER~1) differs from the long path git reports.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@trek-e
trek-e added this pull request to the merge queue Oct 23, 2026
Merged via the queue into next with commit 6834dbe Oct 23, 2026
44 checks passed
@trek-e
trek-e deleted the fix/5519-code-review-files-skips-every-file-on-wi branch October 23, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

code-review --files skips every file on Windows Git Bash (realpath /c/… vs rev-parse C:/…)

1 participant