-
Notifications
You must be signed in to change notification settings - Fork 0
feat: core/drift.ts + core/differ.ts — three-way merge engine (#11) #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
09d73b5
feat(renderer): export renderString helper (#11)
breferrari 06de8e2
feat: core/drift.ts + core/differ.ts — three-way merge engine (#11)
breferrari 290bdcc
docs(roadmap): tick merge engine; defer orphan detection to v0.2 (#11…
breferrari c905719
refactor: dedupe ENOENT dance, parallelize drift reads, share state f…
breferrari 31c9e04
fix(differ): normalize CRLF in three-way merge inputs
breferrari 3bb4623
ci: run matrix on ubuntu + windows + macos
breferrari e4e9195
chore(gitignore): exclude Claude Code's ScheduleWakeup lock file
breferrari ae0d818
refactor: tighten merge engine per self-review
breferrari b5462e3
refactor(errors): type-safe ErrorCode union for ShardMindError
breferrari e5304dd
feat(drift): land orphan detection in v0.1 (closes #47)
breferrari 79d4651
refactor: review-pass cleanup (vault-paths consts, LINE_SPLIT, inline…
breferrari 5a06b8f
refactor: collapse review-pass dismissals after pushback
breferrari 350d25f
test(merge): edge-case fixtures — empty file, UTF-8, frontmatter merge
breferrari cabe217
docs(spec): §4.8 and §4.9 reflect implementation reality
breferrari 03248fe
feat: property-based invariants + fix node-diff3 prototype-key crash
breferrari d900d42
hardening: turn the merge engine upside down, fix 3 real bugs
breferrari 45337c0
chore(gitignore): exclude local fork clones (e.g. node-diff3)
breferrari 3de06de
Revert "chore(gitignore): exclude local fork clones (e.g. node-diff3)"
breferrari 430c2d2
docs: track the node-diff3 workaround removal as a followup (#49)
breferrari df54469
fix: Copilot review round 2 — nine real findings
breferrari File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,204 @@ | ||
| /** | ||
| * Three-way merge engine. | ||
| * | ||
| * Given the old template, the new template, and the file on disk, | ||
| * `computeMergeAction` decides whether to skip, silently overwrite, | ||
| * auto-merge, or surface a conflict. When a merge is needed, the heavy | ||
| * lifting is delegated to node-diff3's Khanna–Myers algorithm (same | ||
| * approach git uses). | ||
| * | ||
| * See docs/IMPLEMENTATION.md §4.9 for the spec. | ||
| */ | ||
|
|
||
| import { diff3MergeRegions, type IRegion, type IUnstableRegion } from 'node-diff3'; | ||
| import type { | ||
| MergeAction, | ||
| MergeResult, | ||
| ConflictRegion, | ||
| RenderContext, | ||
| } from '../runtime/types.js'; | ||
| import { ShardMindError } from '../runtime/types.js'; | ||
| import { sha256 } from './fs-utils.js'; | ||
| import { renderString } from './renderer.js'; | ||
|
|
||
| const CONFLICT_START = '<<<<<<< yours'; | ||
| const CONFLICT_SEPARATOR = '======='; | ||
| const CONFLICT_END = '>>>>>>> shard update'; | ||
|
|
||
| export interface ComputeMergeActionInput { | ||
| readonly path: string; | ||
| readonly ownership: 'managed' | 'modified'; | ||
| readonly oldTemplate: string; | ||
| readonly newTemplate: string; | ||
| readonly oldValues: Record<string, unknown>; | ||
| readonly newValues: Record<string, unknown>; | ||
| readonly actualContent: string; | ||
| readonly renderContext: RenderContext; | ||
| } | ||
|
|
||
| export interface ThreeWayMergeResult { | ||
| readonly content: string; | ||
| readonly conflicts: ConflictRegion[]; | ||
| readonly stats: MergeResult['stats']; | ||
| } | ||
|
|
||
| export async function computeMergeAction( | ||
| input: ComputeMergeActionInput, | ||
| ): Promise<MergeAction> { | ||
| const base = renderString( | ||
| input.oldTemplate, | ||
| { ...input.renderContext, values: input.oldValues }, | ||
| input.path, | ||
| ); | ||
| const ours = renderString( | ||
| input.newTemplate, | ||
| { ...input.renderContext, values: input.newValues }, | ||
| input.path, | ||
| ); | ||
|
|
||
| if (sha256(base) === sha256(ours)) { | ||
| return { type: 'skip', reason: 'no upstream change' }; | ||
| } | ||
|
|
||
| if (input.ownership === 'managed') { | ||
| return { type: 'overwrite', content: ours }; | ||
| } | ||
|
|
||
| const merge = runMerge(base, input.actualContent, ours, input.path); | ||
|
|
||
| if (merge.conflicts.length === 0) { | ||
| return { | ||
| type: 'auto_merge', | ||
| content: merge.content, | ||
| stats: { | ||
| linesUnchanged: merge.stats.linesUnchanged, | ||
| linesAutoMerged: merge.stats.linesAutoMerged, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| type: 'conflict', | ||
| result: { | ||
| content: merge.content, | ||
| hasConflicts: true, | ||
| conflicts: merge.conflicts, | ||
| stats: merge.stats, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| function runMerge(base: string, theirs: string, ours: string, path: string): ThreeWayMergeResult { | ||
| try { | ||
| return threeWayMerge(base, theirs, ours); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| throw new ShardMindError( | ||
| `Three-way merge failed for ${path}: ${message}`, | ||
| 'MERGE_FAILED', | ||
| 'Re-run with --verbose for the full trace, then report at github.com/breferrari/shardmind/issues.', | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Line-based three-way merge. `a` is theirs (user on disk), `o` is base | ||
| * (rendered from old template + old values), `b` is ours (rendered from | ||
| * new template + new values). Convention matches diff3MergeRegions and | ||
| * the git conflict-marker vocabulary (`<<<<<<< yours` wraps theirs, | ||
| * `>>>>>>> shard update` wraps ours). | ||
| */ | ||
| export function threeWayMerge( | ||
| base: string, | ||
| theirs: string, | ||
| ours: string, | ||
| ): ThreeWayMergeResult { | ||
| // Normalize line endings to LF before splitting. `base` and `ours` are | ||
| // renderer output (always LF), but `theirs` comes from disk and may be | ||
| // CRLF on Windows. Without this, every line in theirs would trail with | ||
| // '\r', producing spurious conflicts. Merged output is LF; callers that | ||
| // need platform-native line endings convert at the write boundary. | ||
| const regions: IRegion<string>[] = diff3MergeRegions( | ||
| theirs.split(/\r?\n/), | ||
| base.split(/\r?\n/), | ||
| ours.split(/\r?\n/), | ||
| ); | ||
|
|
||
| const merged: string[] = []; | ||
| const conflicts: ConflictRegion[] = []; | ||
| const stats = { linesUnchanged: 0, linesAutoMerged: 0, linesConflicted: 0 }; | ||
|
|
||
| for (const region of regions) { | ||
| if (region.stable) { | ||
| merged.push(...region.bufferContent); | ||
| // Stable region with buffer === 'o' means all three buffers agreed | ||
| // (truly unchanged). buffer === 'a' or 'b' means diff3 resolved to | ||
| // one side's version without ambiguity — count those as auto-merged. | ||
| if (region.buffer === 'o') { | ||
| stats.linesUnchanged += region.bufferContent.length; | ||
| } else { | ||
| stats.linesAutoMerged += region.bufferContent.length; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| const resolution = resolveUnstableRegion(region, merged.length); | ||
| merged.push(...resolution.lines); | ||
| if (resolution.conflict) conflicts.push(resolution.conflict); | ||
| stats.linesAutoMerged += resolution.autoMergedLines; | ||
| stats.linesConflicted += resolution.conflictedLines; | ||
| } | ||
|
|
||
| return { content: merged.join('\n'), conflicts, stats }; | ||
| } | ||
|
|
||
| interface RegionResolution { | ||
| readonly lines: readonly string[]; | ||
| readonly conflict: ConflictRegion | null; | ||
| readonly autoMergedLines: number; | ||
| readonly conflictedLines: number; | ||
| } | ||
|
|
||
| /** | ||
| * Classify one unstable region. If either side kept the base unchanged (or | ||
| * both sides made the identical change), we can auto-merge. Otherwise we | ||
| * emit git-style conflict markers and describe a `ConflictRegion` for the | ||
| * UI layer. | ||
| * | ||
| * Pure function — no mutation. `mergedLengthBefore` is the length of the | ||
| * output buffer prior to this region and is used only to compute the | ||
| * 1-indexed line range recorded in the ConflictRegion. | ||
| */ | ||
| function resolveUnstableRegion( | ||
| region: IUnstableRegion<string>, | ||
| mergedLengthBefore: number, | ||
| ): RegionResolution { | ||
| const { aContent: theirs, bContent: ours, oContent: base } = region; | ||
|
|
||
| if (arraysEqual(theirs, base)) { | ||
| return { lines: ours, conflict: null, autoMergedLines: ours.length, conflictedLines: 0 }; | ||
| } | ||
| if (arraysEqual(ours, base) || arraysEqual(theirs, ours)) { | ||
| return { lines: theirs, conflict: null, autoMergedLines: theirs.length, conflictedLines: 0 }; | ||
| } | ||
|
|
||
| const lines = [CONFLICT_START, ...theirs, CONFLICT_SEPARATOR, ...ours, CONFLICT_END]; | ||
| const lineStart = mergedLengthBefore + 1; | ||
| const lineEnd = mergedLengthBefore + lines.length; | ||
| return { | ||
| lines, | ||
| conflict: { | ||
| lineStart, | ||
| lineEnd, | ||
| base: base.join('\n'), | ||
| theirs: theirs.join('\n'), | ||
| ours: ours.join('\n'), | ||
| }, | ||
| autoMergedLines: 0, | ||
| conflictedLines: theirs.length + ours.length, | ||
| }; | ||
| } | ||
|
|
||
| function arraysEqual<T>(a: readonly T[], b: readonly T[]): boolean { | ||
| return a.length === b.length && a.every((v, i) => v === b[i]); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
threeWayMerge() splits on /\r?\n/, which will produce a trailing "" element when the input ends with a newline. That extra sentinel line gets counted into stats and affects ConflictRegion lineStart/lineEnd, so line accounting can be off-by-one (and tests currently allow this via >= checks). Consider normalizing to a line array that strips the final empty element while separately preserving whether the original had a trailing newline, then re-append the newline when joining the merged output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the stats inflation. Fixed in df54469 — when all three inputs end with
\n,split(/\r?\n/)produces a trailing "" that diff3 emits as a stable region and inflateslinesUnchangedby 1. After the merge loop, we now detect this and subtract one fromlinesUnchanged.Kept the trailing "" flowing through diff3 itself rather than stripping it — a newline-status difference between theirs and ours is a legitimate auto-merge concern, not a pre-merge normalization. My first attempt stripped + re-appended based on ours' style, which broke the property test "theirs-only-changed ⇒ output equals theirs". The stats-only correction preserves merge semantics and fixes the off-by-one in reporting.