Add UI-based merge conflict resolution

[pedrogaudencio]

• add conflicts tab to change request menu/view
• add active state styling to conflicts tab in change request menu
• create conflicts template for change request view
• use base branch commit instead of merge base for change request conflict resolution
• add resolve box/selection for each conflict
• add submit flow to commit changes
• UI tweaks
• add tests

Closes #153

[pieer]

/crush_fast {{openrouter, google/gemini-3.1-pro-preview, google/gemini-3-flash-preview}}

---

AI review started.

[github-actions]

Advanced AI Review

• Type: Agentic (crush)
• Model: google/gemini-3.1-pro-preview

Click to expand review

1. 🔴 Conflict wrappers are appended to the bottom of the table, breaking file order

• Addressed
• Dismissed

In web_src/js/features/repo-conflict-review.ts, you correctly capture the container's parent node to insert the wrapper into the table. However, since the script moves all group rows inside innerTbody.append(row) before inserting the wrapperRow, the firstRow becomes detached from parentNode. Consequently, parentNode.insertBefore(wrapperRow, null) acts purely like appendChild and pushes all conflicts to the very bottom of the document tree, scrambling the file's visual diff sequence.

To fix this, insert the wrapperRow into the DOM before the inner loop detaches the conflict rows.

web_src/js/features/repo-conflict-review.ts:69-79

    const firstRow = group[0];
    const parentNode = firstRow.parentNode;

    // Create wrapper container row
    const wrapperRow = document.createElement('tr');
    wrapperRow.className = 'conflict-wrapper-row';
    
    // Insert wrapper into the table tree BEFORE we detach the conflict lines
    if (parentNode) {
      parentNode.insertBefore(wrapperRow, firstRow);
    }

    const wrapperCell = document.createElement('td');
    wrapperCell.colSpan = 6;

Be sure to remove the faulty insert statement at the end of the block:
web_src/js/features/repo-conflict-review.ts:181-183

-    // Insert wrapper before the first conflict line's original position
-    parentNode.insertBefore(wrapperRow, null);

2. 🔴 Unmatched additions and deletions are hidden as context lines instead of conflicts

• Addressed
• Dismissed

In custom/templates/repo/diff/conflicts_section_split.tmpl, the condition {{if or (ne .GetType 2) (not $hasmatch)}} strictly scopes conflicts to DEL rows that have an exact, matching ADD counterpart. If an author writes a completely new paragraph (.GetType == 2) or removes a chunk without replacing it (.GetType == 3 without a match), the code delegates those changes to the {{else}} block where they are hardcoded as data-line-type="context" and CSS class same-code.

Because your JS gathers conflicts based on data-line-type="conflict", these changes will bypass the conflict resolver interface entirely and be visually folded/hidden by the "Fold" button, enabling users to silently weave unresolved additions and deletions into the codebase.

Separate actual identical context lines (.GetType == 1) from single-side changes (.GetType == 2 or 3).

custom/templates/repo/diff/conflicts_section_split.tmpl:56-80

			{{else if eq .GetType 1}}
				{{/* Context line (same on both sides) */}}
				{{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale}}
				<tr class="context-line same-code nl-{{$k}} ol-{{$k}}" data-line-type="context">
					<td class="lines-num lines-num-old" data-line-num="{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}"><span>{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}</span></td>
					<td class="lines-type-marker lines-type-marker-old">{{if $line.LeftIdx}}<span class="tw-font-mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span>{{end}}</td>
					<td class="lines-code lines-code-old">
						{{- if $line.LeftIdx -}}
							{{- template "repo/diff/section_code" dict "diff" $inlineDiff -}}
						{{- else -}}
							<code class="code-inner"></code>
						{{- end -}}
					</td>
					<td class="lines-num lines-num-new" data-line-num="{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}"><span>{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}</span></td>
					<td class="lines-type-marker lines-type-marker-new">{{if $line.RightIdx}}<span class="tw-font-mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span>{{end}}</td>
					<td class="lines-code lines-code-new">
						{{- if $line.RightIdx -}}
							{{- template "repo/diff/section_code" dict "diff" $inlineDiff -}}
						{{- else -}}
							<code class="code-inner"></code>
						{{- end -}}
					</td>
				</tr>
			{{else}}
				{{/* Single-side change (unmatched DEL or unmatched ADD) */}}
				{{$inlineDiff := $section.GetComputedInlineDiffFor $line ctx.Locale}}
				<tr class="conflict-line {{if eq .GetType 2}}add-code{{else}}del-code{{end}} nl-{{$k}} ol-{{$k}}" data-line-type="conflict">
					<td class="lines-num lines-num-old" data-line-num="{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}"><span>{{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}}</span></td>
					<td class="lines-type-marker lines-type-marker-old {{if eq .GetType 3}}del-code{{end}}">{{if $line.LeftIdx}}<span class="tw-font-mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span>{{end}}</td>
					<td class="lines-code lines-code-old {{if eq .GetType 3}}del-code{{end}}">
						{{- if $line.LeftIdx -}}
							{{- template "repo/diff/section_code" dict "diff" $inlineDiff -}}
						{{- else -}}
							<code class="code-inner"></code>
						{{- end -}}
					</td>
					<td class="lines-num lines-num-new {{if eq .GetType 2}}add-code{{end}}" data-line-num="{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}"><span>{{if $line.RightIdx}}{{$line.RightIdx}}{{end}}</span></td>
					<td class="lines-type-marker lines-type-marker-new {{if eq .GetType 2}}add-code{{end}}">{{if $line.RightIdx}}<span class="tw-font-mono" data-type-marker="{{$line.GetLineTypeMarker}}"></span>{{end}}</td>
					<td class="lines-code lines-code-new {{if eq .GetType 2}}add-code{{end}}">
						{{- if $line.RightIdx -}}
							{{- template "repo/diff/section_code" dict "diff" $inlineDiff -}}
						{{- else -}}
							<code class="code-inner"></code>
						{{- end -}}
					</td>
				</tr>
			{{end}}

3. 🟡 Active state styling is missing on the Conflicts tab

• Addressed
• Dismissed

The PR description states "add active state styling to conflicts tab in change request menu". However, the anchor tag for the new Conflicts tab lacks the conditional {{if .PageIsPullConflicts}}active{{end}} check that the other tabs use.

custom/templates/repo/pulls/tab_menu.tmpl:7-11

        {{if .IsPullFilesConflicted}}
        <a class="item {{if .PageIsPullConflicts}}active{{end}}" href="{{.Issue.Link}}/conflicts">
            {{svg "octicon-git-merge"}}
            {{template "shared/misc/tabtitle" (ctx.Locale.Tr "repo.pulls.tab_conflicts")}}
        </a>
        {{end}}

4. 🟡 Incorrect and mismatching backgrounds for right-side code

• Addressed
• Dismissed

In custom/public/assets/css/repo.custom.css, there are hardcoded CSS overrides. Currently, the color for .lines-code-new (intended as the right-side addition block) uses #ffe7d1 (a light orange tint) rather than the intended #e5e9ff blue color used accurately in the gutter directly above it. Using hardcoded colors also actively breaks readability when standard Gitea dark mode is invoked.

It is better to rely on var(--color-diff-added-row-bg) and var(--color-diff-removed-row-bg), but if you need strictly overriding the hardcoded values:

custom/public/assets/css/repo.custom.css:262-264

 /* Right code content: match gutter #FFE7D1 */
 .conflict-inner-table .add-code .lines-code-new {
-  background: #ffe7d1 !important;
+  background: #e5e9ff !important;
 }

5. ⚪️ <template> elements should be strictly cloned via .content instead of .innerHTML

• Addressed
• Dismissed

In web_src/js/features/repo-conflict-review.ts, parsing the inner elements of a standard <template> tag using .innerHTML can be technically unpredictable in specific browser edge cases. Best practices call for structurally cloning the nodes context to preserve strict fragment functionality (essential for isolated component scripts).

web_src/js/features/repo-conflict-review.ts:153-156

    if (editorTemplate) {
      // Clone the editor template content
      const editorContent = document.createElement('div');
      editorContent.className = 'conflict-comment-form';
      editorContent.appendChild(editorTemplate.content.cloneNode(true));

6. ⚪️ Consider allowing users to un-resolve or change their selection

• Addressed
• Dismissed

Currently, clicking the "Resolve" button permanently forces keepBtn.disabled = true; and useBtn.disabled = true; leaving users stranded with no way to revert or toggle resolving states without doing a hard page refresh. Implementing a basic "undo" flow or a simple toggle structure for the resolve mechanism would improve UX usability.

---

Review generated using google/gemini-3.1-pro-preview via OpenRouter. Comment /crush_fast {{openrouter, google/gemini-3.1-pro-preview, google/gemini-3-flash-preview}} to re-run.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

web_src/js/features/repo-conflict-review.ts:605

• The submit flow uses hard-coded UI/toast strings (e.g. “Submitting…”, “Submit failed: …”, “network error”). These should be localized (via window.config.i18n or template-provided strings) to keep the conflicts flow consistent with the rest of the app’s i18n behavior.

      setButtonsState(true, 'Submitting…');

      try {
        const resp = await POST(window.location.pathname, {data: {baseCommitID, headCommitID, files}});
        if (resp.ok) {
          window.location.href = issueLink;
        } else {
          let msg: string;
          try {
            msg = await resp.text();
          } catch {
            msg = `HTTP ${resp.status}`;
          }
          setButtonsState(false);
          showErrorToast(`Submit failed: ${msg}`);
        }
      } catch {
        setButtonsState(false);
        showErrorToast('Submit failed: network error');
      }

[pedrogaudencio]

Advanced AI Review

• Type: Agentic (auggie)
• Model: openai/gpt-5.5

Click to expand review

1. 🔴 Modify/delete conflicts are resolved as empty files, not deletions

• Addressed
• Dismissed

The conflict-resolution payload only sends resolved text for each group (web_src/js/features/repo-conflict-review.ts:561-579), so the backend cannot distinguish “the user chose the deleted side” from “the user wants this file to become empty”. In the delete-conflict path, SubmitConflictResolution treats a missing head-side file as empty content and a regular blob (routers/web/repo/pull.go:2026-2039), then always hashes rf.content and calls ResolveIndexPath (routers/web/repo/pull.go:2189-2200). ResolveIndexPath always installs a stage-0 blob (services/repository/files/temp_repo.go:303-323).

That means a modify/delete conflict cannot be resolved by deleting the file. Choosing the deleted side produces a zero-byte file in the merge commit, which is data corruption and is especially surprising because Git’s native resolution would remove the path from the index.

Suggested direction: carry deletion intent through the UI/API and remove the path from the index instead of writing an empty blob when the selected resolution is deletion. For example:

type resolvedFile struct {
    path    string
    mode    string
    content []byte
    delete  bool
}

for _, rf := range resolvedFiles {
    if rf.delete {
        if err := t.RemoveFilesFromIndex(ctx, rf.path); err != nil {
            ctx.ServerError("RemoveFilesFromIndex", err)
            releaser()
            return
        }
        continue
    }
    // existing hash + ResolveIndexPath path
}

This also needs a test that creates a real modify/delete conflict and verifies that choosing the deleted side removes the file from the resulting merge commit rather than creating an empty blob.

2. 🟡 Non-conflicting hunks in conflicted files are treated as conflicts and can be dropped

• Addressed
• Dismissed

The backend uses a real merge only to determine which files are unmerged (routers/web/repo/pull.go:1415-1453, routers/web/repo/pull.go:2148-2167), but both the GET and POST conflict groups are derived from a plain two-way diff between baseCommitID and headCommitID (routers/web/repo/pull.go:1505-1514, routers/web/repo/pull.go:2063-2072). The template then labels every add/delete row in those files as data-line-type="conflict" (custom/templates/repo/diff/conflicts_section_split.tmpl:79-101).

For a file that has one true merge conflict plus another base-only change that Git would auto-merge cleanly, the base-only hunk is shown as another “conflict” and is included in the text-based replacement process. If the user clicks “Use this” for that hunk, the final blob is built from the original head content and the auto-mergeable base-side change is discarded (routers/web/repo/pull.go:2108, routers/web/repo/pull.go:2189-2200). This contradicts the comment that the real merge index keeps “non-conflicting base-side changes” (routers/web/repo/pull.go:2120-2124) for changes inside the same conflicted file.

Suggested direction: render and submit actual merge conflict hunks, not every base-vs-head diff hunk in an unmerged file. A safer approach is to use the post-merge working-tree file with conflict markers as the source of truth, expose only marker blocks as resolvable groups, and replace those marker blocks while preserving the rest of Git’s auto-merged content. Add a regression test where the same file contains one true conflict and one separate base-only edit; resolving the conflict should preserve the base-only edit without requiring the user to manually choose it.

3. 🟡 The final push bypasses pre-receive/post-receive hooks and branch protection

• Addressed
• Dismissed

SubmitConflictResolution pushes the merge commit with useInternalEnv=true (routers/web/repo/pull.go:2239). PushWithOptions maps that to InternalPushingEnvironment (services/repository/files/temp_repo.go:501-508), and that environment explicitly disables hook processing (modules/repository/env.go:41-48). This is risky for a normal branch update: pre-receive checks, protected-branch enforcement, post-receive push processing, webhooks, branch caches, and other side effects are skipped.

The permission check also grants an unconditional bypass to the PR poster or a site admin (routers/web/repo/pull.go:1890-1896). Even though IsUserAllowedToUpdate accounts for head-branch permissions/protection, the isPosterOrAdmin branch can still reach the internal push path without the equivalent protected-branch checks. This can let conflict resolution update a protected head branch in a way a normal push would reject.

Suggested direction: push this branch update through the normal hook path, similar to existing pull-update merges, and use a push trigger where needed. If an internal push is absolutely required, duplicate the relevant pre-receive checks before pushing and explicitly enqueue the same post-receive side effects that are skipped.

if err := t.PushWithOptions(ctx, ctx.Doer, mergeCommitID, pull.HeadBranch, false); err != nil {
    // preserve existing out-of-date/rejected handling
}

4. ⚪️ POST recomputes conflict indices with a different whitespace mode than GET

• Addressed
• Dismissed

The GET route for /conflicts runs SetWhitespaceBehavior, so the rendered conflict groups can respect the user’s saved or query-selected whitespace mode (routers/web/web.go:1249, routers/web/web.go:1628). The POST route does not run that middleware (routers/web/web.go:1250, routers/web/web.go:1629), but SubmitConflictResolution still reads ctx.Data["WhitespaceBehavior"] before recomputing the diff used to validate indices (routers/web/repo/pull.go:2063-2072). On POST this value is unset, so it falls back to the default show-all diff.

For users whose conflict page was rendered with ignore-all, ignore-change, or ignore-eol, the frontend sends indices based on one grouping while the backend validates against another. That can produce spurious “missing resolution” / “index out of range” errors or apply the submitted text to a different group.

Suggested direction: either force the conflict page to always use show-all on both GET and POST, or add SetWhitespaceBehavior to the POST route and include the exact rendered whitespace mode in the submit payload so the backend recomputes the same grouping.

[taoeffect]

Awesome! @pedrogaudencio could you please run one more test to see what happens in the following scenario:

1. Article created by UserA
2. UserB sends in a PR to modify it
3. UserC sends in a PR to modify it
4. UserA accepts and merges UserC's PR that will cause a conflict with UserB's PR
5. UserD sends in a PR on top of the newly merged changes that also conflict with UserB's PR
6. UserA accepts and merges UserD's PR

What happens now? 2 conflicting PRs have been merged. Does UserB's PR handle this gracefully? Or is it broken?

[pieer]

@taoeffect @pedrogaudencio : Looked into what happens when multiple conflicting PRs get merged before a pending PR is resolved. Here's the situation and some options for how we could improve it.

---

The Scenario

1. UserA creates an article.
2. UserB, UserC, and UserD all send PRs that edit it.
3. UserA merges UserC's PR → conflict with UserB's PR is flagged. ✅
4. UserA merges UserD's PR (also conflicts with UserB's) → conflict is re-flagged. ✅
5. UserB (or UserA) opens UserB's conflict resolution page.

How It's Handled Today

The backend is solid. When the conflict page loads, it always runs a fresh git merge against the current base branch — so it naturally picks up all accumulated changes from both UserC and UserD at once. There's no "double conflict" bug; it all resolves into a single coherent conflict review.

The problem is a race condition on submission:

• The page records the base commit SHA at render time.
• If the base moves between page load and submission (e.g. another PR merges in the meantime), the backend rejects the submission with a 409 and the message: "base branch has been updated since you loaded this page, please reload and try again."
• The submit buttons re-enable, and a toast shows the error.
• But: reloading loses all resolved conflicts — they live only in the DOM, with no draft persistence. The new page may also show a different set of conflicts entirely.

---

Improvement Options

Option A — Better messaging only (already done)
Update the toast to explicitly warn: "Your resolutions have not been saved — you will need to redo them after reloading." Low effort, sets expectations.

Option B — sessionStorage draft
Before submitting, serialize all resolutions to sessionStorage. On page load, if a draft exists for the same PR + head commit, offer to restore it. Works for simple reloads; breaks if the conflict set changes significantly after a new merge (restored choices
might no longer map to the right hunks).

Option C — Server-side draft
Autosave resolutions to the server as the user works (e.g. a PATCH /conflicts/draft endpoint). On page load, restore the draft if the head commit still matches. Survives browser crashes, tab switches, and device changes. More implementation work.

Option D — Optimistic merge with conflict re-detection
On 409, don't reject outright — instead recompute which of the user's resolutions are still valid (hunks that haven't changed), pre-apply them, and only ask the user to re-resolve the hunks that actually changed. The most seamless UX but significantly more
complex on both the diff and frontend sides.

---

Happy to hear preferences — even just A+B would cover most real-world cases with minimal effort.