fix(merger): address code-review findings on autostash + observer
P0 — parsePorcelainZ rename/copy handling Git's -z porcelain emits `R <new>\0<old>\0` for renames (and C for copies). The naive split-and-slice treated <old> as an independent dirty path, which made runObservedDestructiveSyncOp warn about phantom "cleared paths" whenever a rename was in flight. Now we detect R/C status and skip the trailing entry. P1 — race-rescue loop unstages between attempts `git stash create` snapshots the index without clearing it, so iteration 2's `git add -A` would re-stage atop iteration 1's leftovers. Tree differences inside the loop then reflected stale staging rather than genuine new writes. Added a `git reset` at the top of each iteration so every attempt starts from a clean index baseline. P1 — writeActiveMergerStatus is now atomic Switched from in-place writeFileSync to temp-file + renameSync. POSIX guarantees rename atomicity on the same filesystem, so a reader can no longer catch the file mid-flush and return a false-negative "no merger active" advisory. P2 — Step regex em-dash clarity `[—\-:]` is functionally fine but obscures intent; switched to `(?:—|-|:)` so the em-dash branch is obvious. Added a test case for the em-dash separator. New tests: - parse-porcelain-z.test.ts (8 cases including renames + copies) - em-dash case added to derive-subject-summary.test.ts 247/247 merger-suite tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
10
.changeset/merger-review-fixes.md
Normal file
10
.changeset/merger-review-fixes.md
Normal file
@@ -0,0 +1,10 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Address code-review findings on the merger autostash work:
|
||||
|
||||
- `parsePorcelainZ` now correctly handles rename/copy entries (`R` / `C` status), which emit two NUL-separated entries for one logical change. Previously the old name was treated as an independent dirty path, causing `runObservedDestructiveSyncOp` to emit spurious "cleared N path(s)" warnings whenever a rename was in flight.
|
||||
- The race-rescue loop in `stashUnrelatedRootDirChanges` now runs `git reset` between attempts so each `git add -A` starts from a clean index, preventing iteration-2+ stashes from drifting due to stale staging rather than genuine new writes.
|
||||
- `writeActiveMergerStatus` now writes the advisory file via temp-path + atomic `renameSync` so dashboard readers can't observe a partial write.
|
||||
- `deriveDeterministicSubjectSummary`'s Step regex switched from `[—\-:]` to `(?:—|-|:)` — same matches, but the em-dash intent is obvious to anyone auditing.
|
||||
Reference in New Issue
Block a user