diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 935bf5c..456bfd8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,6 +21,9 @@ jobs: ai/tests/test-ai-installers.sh ai/tests/test-command-log.sh ai/tests/test-log-step-done.sh + ai/skills/resolve-conflicts/scripts/test-conflict-status.sh + ai/skills/resolve-conflicts/scripts/test-categorize-conflicts.sh + ai/skills/resolve-conflicts/scripts/test-ensure-diff3-markers.sh ai/skills/ran/scripts/tests/test-ran-report.sh ai/helpers/tests/test-repo-context.sh bin/lib/test-git-pr.sh diff --git a/ai/skills/resolve-conflicts/SKILL.md b/ai/skills/resolve-conflicts/SKILL.md index 7215c55..1e777f6 100644 --- a/ai/skills/resolve-conflicts/SKILL.md +++ b/ai/skills/resolve-conflicts/SKILL.md @@ -82,7 +82,19 @@ For non-rebase contexts, omit the step count: > **Conflicts (merge):** -#### 2b: Resolve by Category +#### 2b: Ensure diff3 Conflict Markers + +Step 2d reads the base section of each hunk to tell a stacked-PR duplicate from a real divergence. Git writes that section only when the host sets `merge.conflictStyle` to `diff3` or `zdiff3`, so this script re-creates the markers in diff3 style when the host sets neither. + +The rewrite re-merges each file from its index stages, which costs two things. It relabels the markers `ours` and `theirs`, so a rebase conflict loses the sha and subject of the commit being replayed. It also reverts any resolution already in the file. The script skips a file with no conflict markers, which is where a rerere replay lands, but it cannot detect a half-finished hand edit. Ask the user before running it if they edited a conflict before invoking this skill. + +```bash +scripts/ensure-diff3-markers.sh +``` + +The script prints one `rewrote\t` line per file that gained a base section. It reads the `conflict-marker-size` attribute of each file, so it still finds the markers in a repository that sets one. Report the rewritten files to the user, then continue. + +#### 2c: Resolve by Category Process conflicts in this order: @@ -112,7 +124,7 @@ Run mergiraf as a second pass (it may have already run as a merge driver during mergiraf solve -- --compact --keep-backup=false ``` -After running mergiraf, read the file and check for remaining conflict markers (`<<<<<<<`). If conflict markers remain, proceed with AI analysis (see Step 2c). +After running mergiraf, read the file and check for remaining conflict markers (`<<<<<<<`). If conflict markers remain, proceed with AI analysis (see Step 2d). If mergiraf fully resolves the file (no markers remain), stage it: @@ -122,25 +134,23 @@ git add **4. Other files (`other`)** -Read the file contents and resolve using AI analysis (see Step 2c). - -#### 2c: AI Conflict Analysis +Read the file contents and resolve using AI analysis (see Step 2d). -For conflicts that remain after mergiraf (or for `other` category files), read the file and analyze each conflict hunk. Conflict markers may appear in diff3 style (with a base section) or standard style (without). Handle both: +#### 2d: AI Conflict Analysis -diff3 style (preferred, enabled via `merge.conflictStyle = diff3`): +For conflicts that remain after mergiraf (or for `other` category files), read the file and analyze each conflict hunk. A file step 2b rewrote carries diff3 markers labeled `ours` and `theirs`: ```text -<<<<<<< HEAD +<<<<<<< ours [head_code] ||||||| base [base_code] ======= [incoming_code] ->>>>>>> commit message +>>>>>>> theirs ``` -Standard style (no base section): +A file step 2b skipped keeps the labels the merge wrote, where the last one names the commit. That file carries a base section only if the host already sets `merge.conflictStyle`: ```text <<<<<<< HEAD @@ -150,6 +160,10 @@ Standard style (no base section): >>>>>>> commit message ``` +A skipped path may also hold no markers at all, because the conflict is binary, it is a delete/modify, or rerere replayed a resolution into it. Stage that file as it stands instead of reading it as a hunk. + +Git sets the marker width from the `conflict-marker-size` attribute, so a repository that sets it writes runs other than seven characters long. + **Stacked PR duplicate detection:** When the base section is empty or contains substantially less code than both sides, this often indicates a stacked PR scenario where a sub-PR was merged, duplicating code that also exists in the feature branch. diff --git a/ai/skills/resolve-conflicts/scripts/ensure-diff3-markers.sh b/ai/skills/resolve-conflicts/scripts/ensure-diff3-markers.sh new file mode 100755 index 0000000..7e637c4 --- /dev/null +++ b/ai/skills/resolve-conflicts/scripts/ensure-diff3-markers.sh @@ -0,0 +1,69 @@ +#!/bin/bash +# Rewrite conflicted working-tree files so their markers carry a base section. +# +# Usage: ensure-diff3-markers.sh +# +# Output format (tab-separated, one per line): +# rewrote\t +# +# Git writes the "||||||| " base section only when merge.conflictStyle is +# diff3 or zdiff3. This script re-creates the markers in diff3 style when the +# host sets neither. + +set -euo pipefail + +toplevel=$(git rev-parse --show-toplevel 2>/dev/null) || { + echo "Error: not in a git repository" >&2 + exit 1 +} + +# git diff reports paths from the top of the working tree, so the greps and +# the checkout below resolve them only from there. +cd "$toplevel" + +style=$(git config --get merge.conflictStyle || true) +case "$style" in + diff3|zdiff3) exit 0 ;; +esac + +# The conflict-marker-size attribute sets how many characters a marker runs +# for, and git accepts any value down to 1. A fixed seven-character pattern +# would miss a shorter marker and report the file as having none. +marker_size() { + local size + size=$(git check-attr conflict-marker-size -- "$1" 2>/dev/null | sed 's/.*: //') + case "$size" in + [1-9]|[1-9][0-9]*) echo "$size" ;; + *) echo 7 ;; + esac +} + +has_marker() { + local char="$1" file="$2" size="$3" + grep -qIE "^[$char]{$size}( |\$)" -- "$file" 2>/dev/null +} + +while IFS= read -r -d '' file; do + size=$(marker_size "$file") + + # A conflicted path with no "<<<<<<<" holds a binary or delete/modify + # conflict, or a resolution that rerere replayed. A rewrite destroys + # that resolution. + if ! has_marker '<' "$file" "$size"; then + continue + fi + if has_marker '|' "$file" "$size"; then + continue + fi + + if ! git checkout --conflict=diff3 -- "$file" 2>/dev/null; then + echo "Warning: could not rewrite conflict markers in $file" >&2 + continue + fi + + # A merge driver named by a gitattributes "merge=" rule re-runs here. + # Its output wins, so the re-checkout does not always add a base section. + if has_marker '|' "$file" "$size"; then + printf "rewrote\t%s\n" "$file" + fi +done < <(git diff --name-only --diff-filter=U -z 2>/dev/null) diff --git a/ai/skills/resolve-conflicts/scripts/test-ensure-diff3-markers.sh b/ai/skills/resolve-conflicts/scripts/test-ensure-diff3-markers.sh new file mode 100755 index 0000000..53d85e1 --- /dev/null +++ b/ai/skills/resolve-conflicts/scripts/test-ensure-diff3-markers.sh @@ -0,0 +1,318 @@ +#!/bin/bash +# Tests for ensure-diff3-markers.sh conflict marker rewriting. +# +# Each test creates a temporary git repository with a real failed merge, so the +# index carries genuine unmerged stages, runs the script, and compares the +# output and the working-tree file. +# +# The host gitconfig may set merge.conflictStyle, which would hide the rewrite +# cases, so every git invocation and the script run see empty global and system +# config. +# +# Usage: test-ensure-diff3-markers.sh + +set -euo pipefail + +export GIT_CONFIG_GLOBAL=/dev/null +export GIT_CONFIG_SYSTEM=/dev/null + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SCRIPT="$SCRIPT_DIR/ensure-diff3-markers.sh" + +passes=0 +failures=0 + +# Prints the path of an empty repository whose temp name starts with "$1". +init_repo() { + local tmp + tmp=$(mktemp -d "${TMPDIR:-/tmp}/$1.XXXXXX") + git -C "$tmp" init -q + git -C "$tmp" config user.name "Test User" + git -C "$tmp" config user.email "test@example.com" + git -C "$tmp" config commit.gpgsign false + echo "$tmp" +} + +# Set before calling setup_repo so the merge itself writes that style's +# markers. Setting the style afterwards leaves two-way markers behind, and the +# script's config gate would then be the only thing under test. +conflict_style="" + +# Set before calling setup_repo to write a conflict-marker-size attribute, +# which changes how many characters each marker runs for. +marker_size="" + +# Leaves the repository mid-merge with every named file (default +# conflict.txt) unmerged. Prints the repository path. +setup_repo() { + local files=("${@:-conflict.txt}") + local tmp file + tmp=$(init_repo "ensure-diff3") + if [[ -n "$conflict_style" ]]; then + git -C "$tmp" config merge.conflictStyle "$conflict_style" + fi + if [[ -n "$marker_size" ]]; then + echo "* conflict-marker-size=$marker_size" > "$tmp/.gitattributes" + fi + for file in "${files[@]}"; do + mkdir -p "$(dirname "$tmp/$file")" + printf 'top\nbase line\nbottom\n' > "$tmp/$file" + git -C "$tmp" add -- "$file" + done + git -C "$tmp" commit -q -m "init" + local base + base=$(git -C "$tmp" branch --show-current) + git -C "$tmp" checkout -q -b theirs + for file in "${files[@]}"; do + printf 'top\ntheir line\nbottom\n' > "$tmp/$file" + done + git -C "$tmp" commit -q -a -m "theirs" + git -C "$tmp" checkout -q "$base" + for file in "${files[@]}"; do + printf 'top\nour line\nbottom\n' > "$tmp/$file" + done + git -C "$tmp" commit -q -a -m "ours" + git -C "$tmp" merge -q theirs >/dev/null 2>&1 || true + echo "$tmp" +} + +assert_output() { + local description="$1" + local expected="$2" + local actual="$3" + if [[ "$actual" == "$expected" ]]; then + passes=$((passes + 1)) + else + echo "FAIL: $description" + echo " expected: $(printf '%s' "$expected" | cat -et)" + echo " actual: $(printf '%s' "$actual" | cat -et)" + failures=$((failures + 1)) + fi +} + +# cmp compares the bytes. A $(cat) comparison drops trailing newlines, so it +# passes over a file whose only change is at the end. +assert_unchanged() { + local description="$1" expected="$2" actual="$3" + if cmp -s "$expected" "$actual"; then + passes=$((passes + 1)) + else + echo "FAIL: $description" + diff "$expected" "$actual" | head -10 + failures=$((failures + 1)) + fi +} + +assert_base_section() { + local description="$1" + local file="$2" + local bars + bars=$(printf '|%.0s' $(seq "${marker_size:-7}")) + assert_output "$description" "1" "$(grep -cF "$bars" "$file" || true)" +} + +# --- Styles that leave git without a base section --- + +assert_rewrites_under_style() { + local style="$1" + local file="${2:-conflict.txt}" + local label="${style:-unset} style" + local repo + conflict_style="$style" + repo=$(setup_repo "$file") + conflict_style="" + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "$label reports the rewrite" "$(printf 'rewrote\t%s' "$file")" "$output" + assert_base_section "$label adds a base section" "$repo/$file" + rm -rf "$repo" +} + +test_styles_without_base_section() { + assert_rewrites_under_style "" + assert_rewrites_under_style "merge" +} + +# --- Styles that already give git a base section --- + +assert_skips_under_style() { + local style="$1" + local repo + conflict_style="$style" + repo=$(setup_repo) + conflict_style="" + assert_base_section "$style style writes a base section itself" "$repo/conflict.txt" + cp "$repo/conflict.txt" "$repo/before.snapshot" + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "$style style prints nothing" "" "$output" + assert_unchanged "$style style leaves the file untouched" \ + "$repo/before.snapshot" "$repo/conflict.txt" + rm -rf "$repo" +} + +test_styles_with_base_section() { + assert_skips_under_style "diff3" + assert_skips_under_style "zdiff3" +} + +# --- Unmerged index entry with a resolved working tree (rerere) --- + +test_resolved_working_tree_survives() { + local repo + repo=$(setup_repo) + local resolved=$'top\nour line\ntheir line\nbottom' + printf '%s\n' "$resolved" > "$repo/conflict.txt" + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "resolved working tree prints nothing" "" "$output" + assert_output "resolved content survives" "$resolved" "$(cat "$repo/conflict.txt")" + rm -rf "$repo" +} + +# --- Working tree already carries a base section --- + +test_existing_base_section_skipped() { + local repo + repo=$(setup_repo) + { + echo "top" + echo "<<<<<<< HEAD" + echo "our line" + echo "||||||| merged common ancestors" + echo "base line" + echo "=======" + echo "their line" + echo ">>>>>>> theirs" + echo "bottom" + } > "$repo/conflict.txt" + cp "$repo/conflict.txt" "$repo/before.snapshot" + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "existing base section prints nothing" "" "$output" + assert_unchanged "existing base section survives" \ + "$repo/before.snapshot" "$repo/conflict.txt" + rm -rf "$repo" +} + +# --- Add/add conflict, where the base side is empty --- + +test_add_add_conflict() { + local repo + repo=$(init_repo "ensure-diff3-addadd") + git -C "$repo" commit -q --allow-empty -m "init" + local base + base=$(git -C "$repo" branch --show-current) + git -C "$repo" checkout -q -b theirs + printf 'their line\n' > "$repo/added.txt" + git -C "$repo" add -- added.txt + git -C "$repo" commit -q -m "theirs" + git -C "$repo" checkout -q "$base" + printf 'our line\n' > "$repo/added.txt" + git -C "$repo" add -- added.txt + git -C "$repo" commit -q -m "ours" + git -C "$repo" merge -q theirs >/dev/null 2>&1 || true + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "add/add reports the rewrite" "$(printf 'rewrote\tadded.txt')" "$output" + assert_base_section "add/add adds a base section" "$repo/added.txt" + assert_output "add/add leaves the base section empty" "" \ + "$(sed -n '/^|||||||/,/^=======/p' "$repo/added.txt" | sed '1d;$d')" + rm -rf "$repo" +} + +# --- A gitattributes merge driver overrides the diff3 style --- + +test_merge_driver_output_not_reported() { + local repo + repo=$(setup_repo) + git -C "$repo" config merge.fake.name "fake" + git -C "$repo" config merge.fake.driver \ + "printf '<<<<<<< ours\nA\n=======\nB\n>>>>>>> theirs\n' > %A; exit 1" + echo 'conflict.txt merge=fake' > "$repo/.gitattributes" + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "driver output without a base section is not reported" "" "$output" + rm -rf "$repo" +} + +# --- A skipped file does not end the loop --- + +test_skip_does_not_stop_later_files() { + local repo + repo=$(setup_repo "a-skipped.txt" "b-conflict.txt") + printf 'hand resolved\n' > "$repo/a-skipped.txt" + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "the file after a skipped one is still rewritten" \ + "$(printf 'rewrote\tb-conflict.txt')" "$output" + assert_output "the skipped file keeps its resolution" "hand resolved" \ + "$(cat "$repo/a-skipped.txt")" + rm -rf "$repo" +} + +# --- Run from a subdirectory, where git reports paths from the top level --- + +test_runs_from_subdirectory() { + local repo + repo=$(setup_repo "sub/conflict.txt") + local output + output=$(cd "$repo/sub" && bash "$SCRIPT") + assert_output "subdirectory cwd reports the top-level path" \ + "$(printf 'rewrote\tsub/conflict.txt')" "$output" + assert_base_section "subdirectory cwd rewrites the file" "$repo/sub/conflict.txt" + rm -rf "$repo" +} + +# --- A conflict-marker-size attribute shortens every marker --- + +test_short_marker_size() { + local repo + marker_size=3 + repo=$(setup_repo) + local output + output=$(cd "$repo" && bash "$SCRIPT") + assert_output "short markers report the rewrite" "$(printf 'rewrote\tconflict.txt')" "$output" + assert_base_section "short markers gain a base section" "$repo/conflict.txt" + marker_size="" + rm -rf "$repo" +} + +# --- Filename containing a space --- + +test_filename_with_space() { + assert_rewrites_under_style "" "with space.txt" +} + +# --- Outside a git repository --- + +test_outside_repository() { + local dir + dir=$(mktemp -d "${TMPDIR:-/tmp}/ensure-diff3-bare.XXXXXX") + local status=0 + (cd "$dir" && GIT_CEILING_DIRECTORIES="$dir" bash "$SCRIPT" >/dev/null 2>&1) || status=$? + assert_output "outside a repository exits 1" "1" "$status" + rm -rf "$dir" +} + +# --- Run all tests --- + +test_styles_without_base_section +test_styles_with_base_section +test_resolved_working_tree_survives +test_existing_base_section_skipped +test_add_add_conflict +test_merge_driver_output_not_reported +test_skip_does_not_stop_later_files +test_runs_from_subdirectory +test_short_marker_size +test_filename_with_space +test_outside_repository + +# --- Summary --- + +echo "" +echo "Results: $passes passed, $failures failed" +if [[ "$failures" -gt 0 ]]; then + exit 1 +fi