Skip to content

Commit c2eaf8f

Browse files
vorporealoz-agent
andauthored
Improve sync workflow conflict resolution. (#17)
## Description When a cherry-pick fails during sync PR creation, the workflow now creates a **conflict PR** instead of hard-failing with a `PermanentSyncError`. This unblocks the sync pipeline by giving humans (and eventually the conflict-resolution agent) a PR artifact to work with, rather than requiring manual RUNBOOK intervention. The conflict PR: - Commits the raw conflict state (with markers or modify/delete artifacts) so reviewers see exactly what git produced. - Invokes the conflict-resolution agent to produce a separate resolution commit on top (currently broken — fails gracefully; will be fixed in a follow-up). - Adds a `Repo-Sync-Conflict: cherry-pick` trailer, `repo-sync:conflict` label, assigns a reviewer, and sends a Slack notification. - Continues processing remaining commits (they stack on top of the conflict branch). The approve workflow checks for the `Repo-Sync-Conflict` trailer and **never** approves a PR that has it — human approval is unconditionally required. This is a structural guarantee independent of the bot's code being correct. Also threads `escalate_to` through the sync workflow so the conflict handler can determine the right reviewer/fallback team. ## Testing Existing tests pass (251/251). The new code paths (`_handle_cherry_pick_conflict`, approve workflow gate) interact with git and the GitHub API at runtime — manual end-to-end testing against a real repo pair is needed. Co-Authored-By: Oz <oz-agent@warp.dev>
1 parent a20750a commit c2eaf8f

7 files changed

Lines changed: 306 additions & 62 deletions

File tree

.github/workflows/sync.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ jobs:
132132
--default-branch "${{ github.event.repository.default_branch }}" \
133133
--slack-webhook-url "${{ inputs.slack_webhook_url }}" \
134134
--private-to-public-fixup-script "${{ inputs.private_to_public_fixup_script }}" \
135-
--public-to-private-fixup-script "${{ inputs.public_to_private_fixup_script }}"
135+
--public-to-private-fixup-script "${{ inputs.public_to_private_fixup_script }}" \
136+
--escalate-to "${{ inputs.escalate_to }}"
136137
env:
137138
WARP_API_KEY: ${{ secrets.warp_api_key }}

docs/RUNBOOK.md

Lines changed: 64 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33
operational procedures for repo-sync failure scenarios.
44

5-
## cherry-pick failure during public-to-private sync
5+
## cherry-pick conflict during public-to-private sync
66

77
### symptoms
88

9-
the sync workflow fails with a cherry-pick conflict error. the Slack notification reads something like: `repo-sync: cherry-pick failed for <sha> in <repo>`. the watermark does not advance, so all subsequent commits are blocked.
9+
the sync workflow creates a conflict PR with the `repo-sync:conflict` label and `Repo-Sync-Conflict` trailer. the Slack notification reads something like: `repo-sync: cherry-pick conflict for <sha> in <repo>. Conflict PR #<n> created.` a reviewer is automatically assigned.
1010

1111
### cause
1212

@@ -16,95 +16,120 @@ this is rare -- it can only happen when a public commit modifies lines adjacent
1616

1717
### remediation
1818

19-
#### option 1: fix the root cause (preferred)
19+
#### option 1: resolve the conflict PR (preferred)
20+
21+
the conflict PR contains the raw cherry-pick conflict (with conflict markers or modify/delete artifacts) and possibly an agent-proposed resolution commit on top.
22+
23+
1. check out the conflict PR's branch locally
24+
2. resolve the conflicts (edit files to remove conflict markers, or handle modify/delete cases)
25+
3. push the resolution as a new commit
26+
4. approve the PR and merge it
27+
5. subsequent sync PRs in the stack will be restacked automatically
28+
29+
the approve bot will **never** auto-approve a conflict PR (the `Repo-Sync-Conflict` trailer prevents this). human approval is always required.
30+
31+
#### option 2: fix the root cause
2032

2133
adjust the private repo so the cherry-pick context matches:
2234

23-
1. identify the conflicting file and lines from the workflow logs
35+
1. identify the conflicting file and lines from the conflict PR
2436
2. in the private repo, move the nearby `!repo-sync` marker regions so they don't overlap with the public commit's diff context. options:
2537
- move the private-only code to a `private/` directory instead of using inline markers
2638
- rearrange the code so the marker region is further from the modified lines
2739
3. merge the fix into the private repo's default branch
28-
4. the next sync workflow run retries the cherry-pick against the updated `main`. if the context now matches, the cherry-pick succeeds and the pipeline unblocks automatically
40+
4. close the conflict PR. the next sync workflow run retries the cherry-pick against the updated `main`. if the context now matches, the cherry-pick succeeds and creates a clean PR
2941

30-
#### option 2: manually create the sync PR (escape hatch)
42+
#### option 3: manually create the sync PR (escape hatch)
3143

32-
if fixing the root cause isn't practical or is too slow:
44+
if the conflict PR is unusable for some reason:
3345

34-
1. determine the source commit SHA and the sync branch name from the workflow logs (format: `repo-sync/public-to-private/<short-sha>`)
35-
2. check out the private repo locally
36-
3. create the sync branch from the current stack top (or `main` if no stack):
46+
1. close the conflict PR
47+
2. determine the source commit SHA and the sync branch name from the workflow logs (format: `repo-sync/public-to-private/<short-sha>`)
48+
3. check out the private repo locally
49+
4. create the sync branch from the current stack top (or `main` if no stack):
3750
```sh
3851
git checkout -b repo-sync/public-to-private/<short-sha> origin/main
3952
```
40-
4. manually apply the public commit's changes, resolving any context mismatches:
53+
5. manually apply the public commit's changes, resolving any context mismatches:
4154
```sh
4255
git cherry-pick <source-sha>
4356
# resolve conflicts, then:
4457
git cherry-pick --continue
4558
```
46-
5. ensure the commit message includes the `Repo-Sync-Origin` trailer:
59+
6. ensure the commit message includes the `Repo-Sync-Origin` trailer:
4760
```sh
4861
git commit --amend -m "$(git log -1 --format='%B')" -m "Repo-Sync-Origin: <source-repo>@<source-sha>"
4962
```
50-
6. push the branch and create the PR with the correct `Repo-Sync-Origin` trailer in the description
51-
7. the sync workflow's idempotency guard will see the branch exists and skip this commit on the next run, unblocking the pipeline
63+
7. push the branch and create the PR with the correct `Repo-Sync-Origin` trailer in the description
64+
8. the sync workflow's idempotency guard will see the branch exists and skip this commit on the next run, unblocking the pipeline
5265

5366
### key properties
5467

55-
- the watermark does not advance past the failing commit, so no commits are lost
68+
- the watermark does not advance past the conflicting commit, so no commits are lost
69+
- subsequent commits are stacked on top of the conflict PR and will be processed once it merges
5670
- the idempotency guard ensures manual intervention does not conflict with automation
57-
- once the failing commit is handled (by either path), the sync workflow processes all remaining unsynced commits automatically
71+
- once the conflict PR is resolved and merged, the sync workflow processes all remaining unsynced commits automatically
5872

59-
## patch apply failure during private-to-public sync
73+
## cherry-pick conflict during private-to-public sync
6074

6175
### symptoms
6276

63-
the sync workflow fails with a patch apply error. the Slack notification reads something like: `repo-sync: patch apply failed for <sha> in <repo>. Un-synced public changes overlap.` the watermark does not advance, so all subsequent commits are blocked.
77+
the sync workflow creates a conflict PR with the `repo-sync:conflict` label and `Repo-Sync-Conflict` trailer. the Slack notification reads something like: `repo-sync: cherry-pick conflict for <sha> in <repo>. Conflict PR #<n> created.` a reviewer is automatically assigned.
6478

6579
### cause
6680

67-
a private commit modifies lines in a file that also has un-synced public changes nearby. the sync workflow generates a patch (diff between consecutive clean snapshots of the private repo) and applies it to the public repo. if the public repo's state in the affected area differs from the clean snapshot's context (because of un-synced public changes), `git apply` fails.
81+
a private commit modifies lines in a file that also has un-synced public changes nearby. the sync workflow generates a clean delta (diff between consecutive stripped snapshots of the private repo) and cherry-picks it onto the public repo. if the public repo's state in the affected area differs from the clean snapshot's context (because of un-synced public changes), the cherry-pick fails.
6882

6983
this happens when both repos independently modify the same area of a file before the changes have been synced in both directions.
7084

7185
### remediation
7286

73-
#### option 1: merge the pending public-to-private sync PRs first (preferred)
87+
#### option 1: resolve the conflict PR (preferred)
88+
89+
the conflict PR contains the raw cherry-pick conflict (with conflict markers or modify/delete artifacts) and possibly an agent-proposed resolution commit on top.
7490

75-
the most common cause is pending public-to-private sync PRs that haven't merged yet. once they merge, the private repo includes the public changes, and the next sync run generates a patch with the correct context.
91+
1. check out the conflict PR's branch locally
92+
2. resolve the conflicts (edit files to remove conflict markers, or handle modify/delete cases)
93+
3. push the resolution as a new commit
94+
4. approve the PR and merge it
95+
5. subsequent sync PRs in the stack will be restacked automatically
7696

77-
1. check for open public-to-private sync PRs in the private repo
78-
2. merge them (or wait for them to auto-merge)
79-
3. the next private-to-public sync run retries with updated context and should succeed
97+
the approve bot will **never** auto-approve a conflict PR (the `Repo-Sync-Conflict` trailer prevents this). human approval is always required.
8098

81-
#### option 2: manually create the sync PR (escape hatch)
99+
#### option 2: merge pending public-to-private sync PRs
82100

83-
if merging pending sync PRs doesn't resolve the issue:
101+
the most common root cause is pending public-to-private sync PRs that haven't merged yet. once they merge, the private repo includes the public changes, and a subsequent retry would generate a patch with the correct context.
84102

85-
1. determine the source commit SHA and sync branch name from the workflow logs (format: `repo-sync/private-to-public/<short-sha>`)
86-
2. check out the public repo locally
87-
3. create the sync branch from the current stack top (or `main` if no stack):
103+
1. close the conflict PR
104+
2. check for open public-to-private sync PRs in the private repo
105+
3. merge them (or wait for them to auto-merge)
106+
4. the next private-to-public sync run retries with updated context and should succeed
107+
108+
#### option 3: manually create the sync PR (escape hatch)
109+
110+
if the conflict PR is unusable for some reason:
111+
112+
1. close the conflict PR
113+
2. determine the source commit SHA and sync branch name from the workflow logs (format: `repo-sync/private-to-public/<short-sha>`)
114+
3. check out the public repo locally
115+
4. create the sync branch from the current stack top (or `main` if no stack):
88116
```sh
89117
git checkout -b repo-sync/private-to-public/<short-sha> origin/main
90118
```
91-
4. generate the patch locally and apply it with conflict resolution:
92-
```sh
93-
# apply with --3way for merge-style conflict resolution, or apply manually
94-
git apply --3way /path/to/patch.patch
95-
```
96-
5. commit with the `Repo-Sync-Origin` trailer:
119+
5. manually apply the changes with conflict resolution
120+
6. commit with the `Repo-Sync-Origin` trailer:
97121
```sh
98122
git commit -m "repo-sync: sync from private" -m "Repo-Sync-Origin: <source-repo>@<source-sha>"
99123
```
100-
6. push the branch and create the PR with the `Repo-Sync-Origin` trailer in the description
101-
7. the sync workflow's idempotency guard will see the branch exists and skip this commit on the next run
124+
7. push the branch and create the PR with the `Repo-Sync-Origin` trailer in the description
125+
8. the sync workflow's idempotency guard will see the branch exists and skip this commit on the next run
102126

103127
### key properties
104128

105-
- the watermark does not advance past the failing commit, so no commits are lost
129+
- the watermark does not advance past the conflicting commit, so no commits are lost
130+
- subsequent commits are stacked on top of the conflict PR and will be processed once it merges
106131
- the idempotency guard ensures manual intervention does not conflict with automation
107-
- merging pending public-to-private sync PRs is usually sufficient to unblock the pipeline
132+
- merging pending public-to-private sync PRs is usually sufficient to prevent these conflicts
108133

109134
## missing Repo-Sync-Origin trailer on merge commit
110135

docs/TECH-DESIGN.md

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,31 @@ when a workflow run starts, it reads the watermark tag to determine the last-syn
3535
b. compute the diff between the two clean snapshots by committing both into a temporary git repo. if the diff is empty, skip (all changes in this commit were internal-only)
3636
c. check if a PR with head branch `repo-sync/private-to-public/<short-sha>` was previously created (idempotency guard -- prevents duplicates if the workflow crashed and restarted mid-run). if a MERGED PR exists, skip without updating the stack base. if an OPEN PR exists, use its branch as the stack base and skip
3737
d. create a branch `repo-sync/private-to-public/<short-sha>` based on the top of the current stack (or `main` if no stack)
38-
e. apply the delta to the public repo by cherry-picking the diff commit from the temporary repo. cherry-pick uses three-way merge internally, so it handles context mismatches from un-synced public changes gracefully. if the cherry-pick produces no changes (delta already present in the public repo), skip. if it conflicts, fail loudly
38+
e. apply the delta to the public repo by cherry-picking the diff commit from the temporary repo. cherry-pick uses three-way merge internally, so it handles context mismatches from un-synced public changes gracefully. if the cherry-pick produces no changes (delta already present in the public repo), skip. if it conflicts, create a **conflict PR** (see below)
3939
f. amend the commit with a generic message (e.g., `"repo-sync: sync from private"`) and the `Repo-Sync-Origin` trailer. **do not** use the source commit's message, as it could leak private information
4040
g. push the branch. if the branch already exists on the remote (from a previous interrupted run where push succeeded but PR creation failed), verify the content matches and skip the push. hard-fail if the content differs
4141
h. create a PR with the base set to the previous sync branch (or `main`)
4242

4343
the sync workflow does **not** approve or enable auto-merge on any PRs. approval is handled by the separate approve workflow when a PR reaches the bottom of the stack.
4444

45+
**cherry-pick conflict PRs:**
46+
47+
when a cherry-pick fails during sync PR creation (either direction), instead of failing the workflow, the sync creates a conflict PR:
48+
1. commit the raw conflict state as-is (with conflict markers or modify/delete artifacts) using `git add -A` + `git commit`. this gives reviewers full visibility into the raw conflict
49+
2. invoke the conflict-resolution agent to produce a **separate resolution commit** on top. if the agent fails (or is unavailable), the branch is left with just the raw conflict commit
50+
3. push the branch and create the PR with a `Repo-Sync-Conflict: cherry-pick` trailer, the `repo-sync:conflict` label, and a `Repo-Sync-Assigned` trailer
51+
4. request review from the person who merged the source PR (or the fallback team)
52+
5. send a Slack notification with the conflict PR number
53+
6. continue processing remaining unsynced commits (they stack on top of the conflict branch)
54+
55+
the `Repo-Sync-Conflict` trailer is a structural guarantee: the approve workflow checks for it and **never** approves a PR with this trailer. conflict PRs unconditionally require human approval, regardless of mergeability state. this ensures no conflict PR is auto-merged without human review.
56+
4557
**public-to-private:**
4658
1. identify unsynced commits on the public repo's default branch
4759
2. for each unsynced commit (in chronological order):
4860
a. check if a branch `repo-sync/public-to-private/<short-sha>` already exists or a PR with that head branch was previously created (idempotency guard)
4961
b. create a branch `repo-sync/public-to-private/<short-sha>` based on the top of the current stack (or `main` if no stack)
50-
c. cherry-pick the commit, preserving author and message. if the cherry-pick fails (rare -- caused by private-only code overlapping with the public commit's diff context), the workflow **fails loudly** and notifies oncall. see [RUNBOOK.md](RUNBOOK.md) for remediation steps
62+
c. cherry-pick the commit, preserving author and message. if the cherry-pick fails (rare -- caused by private-only code overlapping with the public commit's diff context), create a **conflict PR** (see below)
5163
d. create a PR with the base set to the previous sync branch (or `main`)
5264

5365
### merge strategy
@@ -94,6 +106,7 @@ the workflow is serialized per PR via a concurrency group (`cancel-in-progress:
94106

95107
**decision logic (in order):**
96108
1. **already handled?** if the PR has an existing approval OR a `Repo-Sync-Assigned` trailer → skip
109+
1b. **conflict PR?** if the PR has a `Repo-Sync-Conflict` trailer → skip. these PRs unconditionally require human approval
97110
2. **commit count?** if the PR has ≠ 1 commit → skip. the restack workflow handles restacking (either post-merge or via the `repo-sync:needs-restack` label, which can be added manually or by the escalation cron)
98111
3. **mergeable?** check via GitHub API (with retries for `UNKNOWN`):
99112
- `MERGEABLE` → approve + enable auto-merge (API-only, no git operations)

src/repo_sync/stack/trailers.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
# Trailer prefixes.
1616
_ORIGIN_PREFIX = "Repo-Sync-Origin:"
1717
_ASSIGNED_PREFIX = "Repo-Sync-Assigned:"
18+
_CONFLICT_PREFIX = "Repo-Sync-Conflict:"
1819

1920

2021
@dataclass(frozen=True)
@@ -125,6 +126,22 @@ def format_origin_trailer(repo: str, sha: str) -> str:
125126
return f"{_ORIGIN_PREFIX} {repo}@{sha}"
126127

127128

129+
def parse_conflict(text: str) -> bool:
130+
"""Check if the text contains a Repo-Sync-Conflict trailer.
131+
132+
Returns True if the trailer is present.
133+
"""
134+
for line in text.splitlines():
135+
if line.strip().startswith(_CONFLICT_PREFIX):
136+
return True
137+
return False
138+
139+
140+
def format_conflict_trailer() -> str:
141+
"""Format a Repo-Sync-Conflict trailer line."""
142+
return f"{_CONFLICT_PREFIX} cherry-pick"
143+
144+
128145
def format_assigned_trailer(username: str, timestamp: datetime) -> str:
129146
"""Format a Repo-Sync-Assigned trailer line.
130147

src/repo_sync/workflows/approve_logic.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
from repo_sync.stack.gh_ops import GhOps
2020
from repo_sync.stack.git_ops import GitOps
21-
from repo_sync.stack.trailers import parse_origin
21+
from repo_sync.stack.trailers import parse_conflict, parse_origin
2222
from repo_sync.workflows.sync import determine_sync_reviewer
2323

2424
logger = logging.getLogger(__name__)
@@ -251,6 +251,20 @@ def run_approve(
251251
if check_already_handled(gh, repo, pr_number):
252252
return
253253

254+
# Step 1b: Conflict trailer check. PRs created by the sync workflow
255+
# with a Repo-Sync-Conflict trailer unconditionally require human
256+
# approval. The approve bot must never approve these.
257+
body = gh._run([
258+
"pr", "view", str(pr_number), "--repo", gh.repo,
259+
"--json", "body", "--jq", ".body",
260+
], check=False)
261+
if body and parse_conflict(body):
262+
logger.info(
263+
"PR #%d has Repo-Sync-Conflict trailer. "
264+
"Human approval required. Skipping.", pr_number,
265+
)
266+
return
267+
254268
# Step 2: Commit count check.
255269
commit_count = check_commit_count(gh, pr_number)
256270
logger.info("PR #%d has %d commit(s).", pr_number, commit_count)

src/repo_sync/workflows/cli.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ def cmd_run_sync(args: argparse.Namespace) -> None:
253253
slack_webhook_url=args.slack_webhook_url,
254254
private_to_public_fixup_script=args.private_to_public_fixup_script,
255255
public_to_private_fixup_script=args.public_to_private_fixup_script,
256+
escalate_to=args.escalate_to,
256257
)
257258
except PermanentSyncError as e:
258259
logging.error("%s", e)
@@ -426,6 +427,10 @@ def main() -> None:
426427
"--public-to-private-fixup-script", default="",
427428
help="Optional script to run after cherry-pick for public-to-private sync (not yet implemented).",
428429
)
430+
p.add_argument(
431+
"--escalate-to", default="@oncall-client-primary",
432+
help="GitHub team or user to escalate to on timeout.",
433+
)
429434
p.set_defaults(func=cmd_run_sync)
430435

431436
# escalation-check.

0 commit comments

Comments
 (0)