Skip to content

Commit 46eaa04

Browse files
authored
Better handle modify/delete conflicts. (#19)
## Description When a cherry-pick encounters a modify/delete conflict (one side modifies a file the other deleted), git does not produce conflict markers. The existing `git add -A` step silently resolves these by keeping the file, and the conflict-resolution agent has no signal that a conflict occurred — so it does nothing. This change detects modify/delete conflicts from `git status --porcelain` before staging, writes a `.repo-sync-conflicts.json` manifest to the working tree (not committed), and updates the conflict-resolution skill to consume it. The agent now knows about these conflicts and defaults to honoring the deletion. ## Testing Manual — will be exercised by the next cherry-pick conflict that involves a deleted file.
1 parent c2eaf8f commit 46eaa04

3 files changed

Lines changed: 110 additions & 17 deletions

File tree

.agents/skills/conflict-resolution/SKILL.md

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ you are a merge conflict resolution agent. your job is to resolve git merge con
99

1010
## context
1111

12-
you are checked out on a branch that has conflict markers in its code. this can happen in two ways:
13-
1. **in-progress git operation** (rebase, cherry-pick, or merge): the conflicting files are unresolved in the git index.
14-
2. **committed conflict markers**: the conflict markers were committed as-is (e.g., by the sync workflow to preserve the raw conflict for review). there is no in-progress git operation.
12+
you are checked out on a branch that has conflicts. these can take two forms:
13+
- **text conflicts**: conflict markers (`<<<<<<<` / `=======` / `>>>>>>>`) in file contents. these appear either as unresolved index entries (in-progress git operation) or as committed markers (case 2: the sync workflow committed them as-is for review).
14+
- **modify/delete conflicts**: one side modified a file while the other deleted it. these do **not** produce conflict markers. instead, the sync workflow detects them before staging, records them in a manifest file (`.repo-sync-conflicts.json` in the repo root), and commits the file as-is (kept). the manifest is written to the working tree (not committed) for you to consume.
1515

16-
your job is to resolve the conflict markers and commit the result. the calling workflow handles pushing -- **do not push**.
16+
your job is to resolve all conflicts — both text and modify/delete — and commit the result. the calling workflow handles pushing -- **do not push**.
1717

1818
## environment
1919

@@ -37,7 +37,40 @@ grep -rln --exclude-dir=.git '^<{7}\s' .
3737

3838
this gives you the list of files containing conflict markers.
3939

40-
### 2. read and understand each conflict
40+
### 2. check for modify/delete conflicts
41+
42+
check whether the calling workflow left a modify/delete manifest:
43+
```sh
44+
cat .repo-sync-conflicts.json 2>/dev/null
45+
```
46+
47+
if the file exists, it contains a JSON object like:
48+
```json
49+
{
50+
"context": "Cherry-pick conflict. 'ours' = current branch (target), 'theirs' = cherry-picked commit (source).",
51+
"modify_delete_conflicts": [
52+
{ "path": "src/foo.rs", "deleted_by": "ours" },
53+
{ "path": "src/bar.rs", "deleted_by": "theirs" }
54+
]
55+
}
56+
```
57+
58+
the `context` field explains what `ours` and `theirs` mean for this conflict. `deleted_by` tells you which side deleted the file; the other side modified it. because `git add -A` was run before the commit, the file currently **exists** in the committed tree (the deletion was not honored).
59+
60+
for each modify/delete conflict:
61+
- read the file to understand the modifications.
62+
- look at git log and surrounding context to understand why the other side deleted it.
63+
- **default heuristic**: honor the deletion (`git rm <file>`). the side that deleted the file made an intentional decision, and the modifications from the other side are typically moot. for example, if the target branch deleted a file and the cherry-picked commit only tweaks a few lines in it, the correct resolution is almost certainly to delete the file.
64+
- **exception**: keep the file if the modification is clearly important and independent of the deletion (e.g., the deletion was part of a rename and the modification adds critical new content that should follow the rename). if you keep the file, no action is needed — it is already in the committed tree.
65+
66+
after processing all modify/delete conflicts, **delete the manifest**:
67+
```sh
68+
rm -f .repo-sync-conflicts.json
69+
```
70+
71+
if the manifest does not exist, skip this step — there are no modify/delete conflicts.
72+
73+
### 3. read and understand each text conflict
4174

4275
for each conflicting file:
4376
- read the entire file.
@@ -53,7 +86,7 @@ for each conflicting file:
5386
the `<ref-or-label>` after `<<<<<<<` and `>>>>>>>` tells you which branch or commit each side came from. **read these labels carefully** -- the meaning of "first side" vs. "second side" depends on whether the conflict arose from a `git merge` or a `git rebase` (rebase swaps the sides relative to merge). do not assume which side is "ours" or "theirs" -- always check the labels.
5487
- look at surrounding code and other files in the repository for context on what the correct resolution should be.
5588

56-
### 3. resolve each conflict
89+
### 4. resolve each text conflict
5790

5891
edit each conflicting file to remove all conflict markers and produce the correct merged result. every conflict region must be resolved -- there must be zero `<<<<<<<`, `=======`, or `>>>>>>>` markers remaining in any file.
5992

@@ -62,7 +95,7 @@ after editing, stage each resolved file:
6295
git add <file>
6396
```
6497

65-
### 4. verify: no remaining conflict markers
98+
### 5. verify: no remaining conflict markers
6699

67100
run a search across the entire repository to confirm no conflict markers remain:
68101
```sh
@@ -71,9 +104,9 @@ grep -Ern --exclude-dir=.git '^<{7}([^<]|$)|^={7}([^=]|$)|^>{7}([^>]|$)' .
71104

72105
this pattern matches exactly 7 repeated characters followed by either a non-matching character or end-of-line. the end-of-line alternative is needed because conflict markers (especially `=======`) can appear as bare lines with nothing after them.
73106

74-
if any markers remain, go back to step 3 and resolve them.
107+
if any markers remain, go back to step 4 and resolve them.
75108

76-
### 5. finalize the resolution
109+
### 6. finalize the resolution
77110

78111
the correct command to finalize the resolution depends on which git operation caused the conflict. detect the in-progress operation and use the appropriate command:
79112

src/repo_sync/stack/git_ops.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,31 @@ def conflicting_files(self) -> list[str]:
164164
return result.stdout.strip().splitlines()
165165
return []
166166

167+
def get_modify_delete_conflicts(self) -> list[dict[str, str]]:
168+
"""Return modify/delete conflicts detected from the unmerged index.
169+
170+
Must be called while the cherry-pick (or merge/rebase) is still
171+
in-progress — i.e., before ``git add -A`` resolves the index.
172+
173+
Each returned dict has:
174+
- ``path``: the conflicting file path.
175+
- ``deleted_by``: ``"ours"`` if our side deleted the file
176+
(porcelain ``DU``), or ``"theirs"`` if their side deleted it
177+
(porcelain ``UD``).
178+
"""
179+
result = self._run(["status", "--porcelain"], check=False)
180+
conflicts: list[dict[str, str]] = []
181+
for line in result.stdout.splitlines():
182+
if len(line) < 4:
183+
continue
184+
xy = line[:2]
185+
path = line[3:]
186+
if xy == "DU":
187+
conflicts.append({"path": path, "deleted_by": "ours"})
188+
elif xy == "UD":
189+
conflicts.append({"path": path, "deleted_by": "theirs"})
190+
return conflicts
191+
167192
def archive_to_dir(self, ref: str, target_dir: str) -> None:
168193
"""Extract the tree at a ref into a directory via git archive.
169194

src/repo_sync/workflows/create_sync_prs.py

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -412,20 +412,55 @@ def _handle_cherry_pick_conflict(
412412
optionally invokes the conflict-resolution agent to add a resolution
413413
commit on top, then creates the PR with appropriate trailers and labels.
414414
"""
415-
# Step 1: Commit the raw conflict as-is.
415+
# Step 1: Detect modify/delete conflicts before staging resolves them.
416+
# In a cherry-pick, "ours" = current branch (target), "theirs" =
417+
# cherry-picked commit (source). git add -A will silently keep the
418+
# file in both cases, which may not be the correct resolution.
419+
md_conflicts = peer_git.get_modify_delete_conflicts()
420+
if md_conflicts:
421+
logger.info(
422+
"Detected %d modify/delete conflict(s): %s",
423+
len(md_conflicts),
424+
[c["path"] for c in md_conflicts],
425+
)
426+
427+
# Step 2: Commit the raw conflict as-is.
416428
origin_trailer = f"Repo-Sync-Origin: {source_repo}@{source_sha}"
417429
peer_git.add_all()
418430
peer_git.commit(commit_message, trailers=[origin_trailer])
419431

420-
# Step 2: Invoke the conflict-resolution agent to produce a separate
432+
# Step 3: If there were modify/delete conflicts, write a manifest to
433+
# the working tree so the conflict-resolution agent can act on them.
434+
# The manifest is intentionally NOT committed — it is a transient
435+
# signal consumed and deleted by the agent.
436+
manifest_path = os.path.join(peer_git.repo_dir, ".repo-sync-conflicts.json")
437+
if md_conflicts:
438+
manifest = {
439+
"context": (
440+
"Cherry-pick conflict. "
441+
"'ours' = current branch (target), "
442+
"'theirs' = cherry-picked commit (source)."
443+
),
444+
"modify_delete_conflicts": md_conflicts,
445+
}
446+
with open(manifest_path, "w") as f:
447+
json.dump(manifest, f, indent=2)
448+
f.write("\n")
449+
450+
# Step 4: Invoke the conflict-resolution agent to produce a separate
421451
# resolution commit on top. The agent runs in Docker and sees the
422-
# committed conflict markers (case 2 in the skill).
452+
# committed conflict markers (case 2 in the skill) plus the manifest
453+
# for any modify/delete conflicts.
423454
from repo_sync.workflows.agent import run_conflict_resolution_agent
424455

425456
agent_succeeded = run_conflict_resolution_agent(
426457
repo_dir=peer_git.repo_dir,
427458
context=f"Cherry-pick conflict on branch {sync_branch}.",
428459
)
460+
461+
# Clean up the manifest if the agent left it behind.
462+
if os.path.exists(manifest_path):
463+
os.remove(manifest_path)
429464
if agent_succeeded:
430465
logger.info("Agent produced a resolution commit for %s.", short_sha)
431466
else:
@@ -434,7 +469,7 @@ def _handle_cherry_pick_conflict(
434469
"PR will contain raw conflict markers.", short_sha,
435470
)
436471

437-
# Step 3: Push and create the PR.
472+
# Step 5: Push and create the PR.
438473
conflict_trailer = format_conflict_trailer()
439474
pr_body = append_trailer(pr_body, origin_trailer)
440475
pr_body = append_trailer(pr_body, conflict_trailer)
@@ -452,7 +487,7 @@ def _handle_cherry_pick_conflict(
452487
)
453488
return
454489

455-
# Step 4: Add conflict label.
490+
# Step 6: Add conflict label.
456491
peer_gh._run(
457492
["label", "create", "repo-sync:conflict",
458493
"--color", "D93F0B",
@@ -465,7 +500,7 @@ def _handle_cherry_pick_conflict(
465500
"--add-label", "repo-sync:conflict"],
466501
)
467502

468-
# Step 5: Determine and assign reviewer.
503+
# Step 7: Determine and assign reviewer.
469504
source_gh = GhOps(source_repo, token=os.environ.get("GH_TOKEN"))
470505
reviewer = determine_sync_reviewer(
471506
source_gh=source_gh,
@@ -478,7 +513,7 @@ def _handle_cherry_pick_conflict(
478513
check=False,
479514
)
480515

481-
# Step 6: Add Repo-Sync-Assigned trailer to PR body.
516+
# Step 8: Add Repo-Sync-Assigned trailer to PR body.
482517
assigned_trailer = format_assigned_trailer(reviewer, datetime.now(timezone.utc))
483518
current_body = peer_gh._run(
484519
["pr", "view", str(pr_number), "--repo", peer_gh.repo,
@@ -493,7 +528,7 @@ def _handle_cherry_pick_conflict(
493528
pr_number, short_sha, reviewer,
494529
)
495530

496-
# Step 7: Slack notification.
531+
# Step 9: Slack notification.
497532
msg = (
498533
f"repo-sync: cherry-pick conflict for {source_sha[:7]} in "
499534
f"{peer_gh.repo}. Conflict PR #{pr_number} created."

0 commit comments

Comments
 (0)