node:fs: apply cpSync errorOnExist per entry, not to the destination directory - #33416
Open
robobun wants to merge 3 commits into
Open
node:fs: apply cpSync errorOnExist per entry, not to the destination directory#33416robobun wants to merge 3 commits into
robobun wants to merge 3 commits into
Claude / Claude Code Review
completed
Jul 6, 2026 in 10m 38s
Code review found 1 potential issue
Found 3 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/js/internal/fs/cp-sync.ts:424-426 |
Missing mirror comment on async cp.ts onDir |
Annotations
Check warning on line 426 in src/js/internal/fs/cp-sync.ts
claude / Claude Code Review
Missing mirror comment on async cp.ts onDir
The PR description says "Added a comment on both halves so the asymmetry doesn't read as an oversight", but only `cp-sync.ts` got the comment — `src/js/internal/fs/cp.ts` is not in the diff, and its `onDir()` (lines 317-329) still has the directory-level `errorOnExist` check with no explanation. The mirror comment is arguably more valuable on the async side, where the surprising-looking check remains: a future reader comparing the two files might assume it's the same bug and delete it. Either ad
Loading