install: fix bun patch --commit cache path for git, github, and tarball dependencies - #37124
Closed
robobun wants to merge 6 commits into
Closed
install: fix bun patch --commit cache path for git, github, and tarball dependencies#37124robobun wants to merge 6 commits into
robobun wants to merge 6 commits into
Claude / Claude Code Review
completed
Aug 7, 2026 in 25m 6s
Code review found 1 important issue
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | test/cli/install/bun-patch.test.ts:1003 |
github/git patch --commit still fails on Windows: patch filename contains ':' (unescaped); new tests will fail on Window |
| 🟡 Nit | test/cli/install/bun-patch.test.ts:1067-1069 |
git dependency test: strict expect(stderr).toBe("") on git setup is non-hermetic |
Annotations
Check failure on line 1003 in test/cli/install/bun-patch.test.ts
claude / Claude Code Review
github/git patch --commit still fails on Windows: patch filename contains ':' (unescaped); new tests will fail on Windows CI
The new `github dependency` and `git dependency` tests will fail on Windows CI: now that the cache-path lookup is fixed, `bun patch --commit` reaches `escape_patch_filename`, which escapes `/ \ space \n \r \t` but not `:` — so the patch filename (e.g. `pkg-to-patch@github:testowner%2Ftestrepo#aaaaaaa.patch`) contains a colon, and `renameat_concurrently` to `patches/…` fails on NTFS (`:` is reserved / the ADS separator), producing `error: failed renaming patch file to patches dir` and a nonzero e
Check warning on line 1069 in test/cli/install/bun-patch.test.ts
claude / Claude Code Review
git dependency test: strict expect(stderr).toBe("") on git setup is non-hermetic
nit: the git setup loop spawns with `env: bunEnv` (which inherits `~/.gitconfig` / system gitconfig) and asserts `expect(stderr).toBe("")`, so ambient config like `core.autocrlf=true` (git-for-windows default → "LF will be replaced by CRLF" on `git add -A`) or `commit.gpgsign=true` will fail the strict equality check. Match the harness convention in `test/cli/test/test-changed.test.ts` / `test/js/bun/patch/patch.test.ts` — set `GIT_CONFIG_NOSYSTEM: "1"` + an empty `GIT_CONFIG_GLOBAL` (and/or `-c
Loading