fix(conflict): deterministic JSON merge + --theirs fallback (no more story thrash)#106
Merged
Merged
Conversation
…ergeable file never thrashes a story
Root cause of clipforge/pulsereview hanging for hours: when the LLM returned
commentary instead of merged content for package.json/vitest.config.ts, the
resolver aborted the whole rebase, and the story thrashed through every
escalation tier indefinitely ('tech lead returned commentary, not file content').
Two fixes in internal/engine/conflict_resolver.go (+ internal/git/conflict.go):
- Structural JSON merge for package.json/tsconfig*.json/jsconfig.json/etc:
pulls both index sides (git.ConflictSides :2:/:3:) and deep-unions the objects
(deepMergeJSON) so BOTH sides' deps/scripts/compilerOptions survive — fully
deterministic, no LLM. Lock files excluded; invalid JSON falls through to LLM.
- Deterministic --theirs fallback (git.CheckoutTheirs) when the LLM genuinely
cannot merge any file — surfaced as the new errUnmergeable sentinel (commentary
or leftover markers). The story-branch version is kept and the rebase continues;
the pre-merge QA gate + integration build validate it. API/transport errors
(fatal, capacity, transient) are NOT errUnmergeable, so they still abort/pause
for a clean retry/resume — never silently take a side under an outage.
Also restores the review/qa orphan-recovery regression test (#103 shipped without
one): resume_orphan_recovery_test.go.
Tests: conflict_json_merge_test.go (deep-union, theirs-wins, invalid-JSON),
conflict_sides_test.go (rebase ours=base/theirs=story semantics, CheckoutTheirs),
conflict_fallback_test.go (commentary→fallback succeeds; generic LLM error still
aborts), resume_orphan_recovery_test.go. go build/vet/lint(0)/test all green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
clipforge and pulsereview hung for hours during a 6-app build: when the LLM conflict resolver returned commentary instead of merged content for
package.json/vitest.config.ts, the resolver aborted the entire rebase and the story thrashed through every escalation tier indefinitely (tech lead returned commentary, not file content). Prior fixes (#91/#93/#99) made commentary fail cleanly but never resolved it — so it still deadlocked.Fix (vxd itself — prevents recurrence on any project)
package.json,tsconfig*.json,jsconfig.json,composer.json,.eslintrc.json, … (lock files excluded). Pulls both index sides (git.ConflictSides:2:/:3:) and deep-unions the objects so both sides' dependencies/scripts/compilerOptions survive. Fully deterministic — no LLM. This is exactly the file class that deadlocked.--theirsfallback (git.CheckoutTheirs) when the LLM genuinely can't merge any file — surfaced via the newerrUnmergeablesentinel (commentary or leftover markers). Keeps the story-branch version and continues; the pre-merge QA gate + integration build validate it. API/transport errors (fatal, capacity, transient) are NOTerrUnmergeable→ they still abort/pause for a clean retry/vxd resume, never silently taking a side under an outage.Rebase ours/theirs (pinned by test)
git rebase <upstream>runs from the story worktree → during a conflict ours = base, theirs = story. Keeping the story's work =--theirs.Test plan
conflict_json_merge_test.go— deep-union of deps/scripts, theirs-wins on scalar/type-mismatch, invalid-JSON falls throughconflict_sides_test.go(git pkg) — ours=base/theirs=story semantics,CheckoutTheirsresolves+stagesconflict_fallback_test.go— commentary → fallback succeeds (rebase completes, story version kept); generic LLM error still abortsresume_orphan_recovery_test.go— guards fix: complete transient-failure recovery (tier-column reset + review/qa-stranded recovery) #103go build,go vet,golangci-lint run(0 issues), fullgo test ./...green🤖 Generated with Claude Code