Skip to content

Commit f5dc652

Browse files
authored
Hide visible merge conflict markers in PR review (#154)
* Polish skill detail dialog layout - Add icon, tag chips, and improved command/path presentation - Make dialog content and skills page scrollable within available height * Handle merge conflicts without visible markers - Fall back to ours/theirs content from the git index when conflict markers cannot be parsed - Apply full-file fallback candidates directly during resolution * Pin @pierre/diffs to 1.1.8 - Replace the beta range with the stable 1.1.8 release - Update the lockfile to match the pinned dependency
1 parent 328eaf4 commit f5dc652

3 files changed

Lines changed: 89 additions & 11 deletions

File tree

apps/server/src/prReview/Layers/MergeConflictResolver.ts

Lines changed: 87 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { promises as fsPromises } from "node:fs";
33

44
import { Effect, Layer } from "effect";
55
import type { PrConflictCandidateResolution, PrReviewSummary } from "@okcode/contracts";
6-
import { GitCore } from "../../git/Services/GitCore.ts";
6+
import { GitCore, type GitCoreShape } from "../../git/Services/GitCore.ts";
77
import {
88
MergeConflictResolver,
99
type MergeConflictResolverShape,
@@ -114,15 +114,91 @@ function buildCandidatesForFile(input: {
114114
return candidates;
115115
}
116116

117-
async function readCandidatesForConflicts(cwd: string, conflictedFiles: readonly string[]) {
117+
/**
118+
* Read the "ours" (stage 2) and "theirs" (stage 3) versions of a conflicted
119+
* file directly from the git index. This works even when the working-tree
120+
* copy has no parseable conflict markers (e.g. binary files, diff3 style,
121+
* already-partially-resolved markers, or multiple conflict blocks).
122+
*/
123+
async function buildFallbackCandidatesFromIndex(
124+
gitCore: GitCoreShape,
125+
cwd: string,
126+
relativePath: string,
127+
): Promise<PrConflictCandidateResolution[]> {
128+
const candidates: PrConflictCandidateResolution[] = [];
129+
const tryStage = async (
130+
stage: "2" | "3",
131+
label: "ours" | "theirs",
132+
title: string,
133+
description: string,
134+
) => {
135+
try {
136+
const result = await Effect.runPromise(
137+
gitCore.execute({
138+
operation: "showConflictStage",
139+
cwd,
140+
args: ["show", `:${stage}:${relativePath}`],
141+
allowNonZeroExit: true,
142+
}),
143+
);
144+
if (result.code === 0) {
145+
candidates.push(
146+
buildCandidate({
147+
id: `${relativePath}:${label}`,
148+
path: relativePath,
149+
title,
150+
description,
151+
confidence: "review",
152+
replacement: result.stdout,
153+
}),
154+
);
155+
}
156+
} catch {
157+
// Stage does not exist in the index; skip this side.
158+
}
159+
};
160+
161+
await tryStage(
162+
"2",
163+
"ours",
164+
"Prefer current side (full file)",
165+
"Review-required candidate using the full current-branch version from the git index.",
166+
);
167+
await tryStage(
168+
"3",
169+
"theirs",
170+
"Prefer incoming side (full file)",
171+
"Review-required candidate using the full incoming-branch version from the git index.",
172+
);
173+
174+
return candidates;
175+
}
176+
177+
async function readCandidatesForConflicts(
178+
cwd: string,
179+
conflictedFiles: readonly string[],
180+
gitCore: GitCoreShape,
181+
) {
118182
const candidates: PrConflictCandidateResolution[] = [];
119183
for (const relativePath of conflictedFiles) {
120184
try {
121185
const absolutePath = path.join(cwd, relativePath);
122186
const contents = await fsPromises.readFile(absolutePath, "utf8");
123-
candidates.push(...buildCandidatesForFile({ relativePath, contents }));
187+
const fileCandidates = buildCandidatesForFile({ relativePath, contents });
188+
if (fileCandidates.length > 0) {
189+
candidates.push(...fileCandidates);
190+
} else {
191+
// Marker parsing failed (diff3 style, multiple blocks, etc.) – fall
192+
// back to full-file ours/theirs from the git index.
193+
candidates.push(
194+
...(await buildFallbackCandidatesFromIndex(gitCore, cwd, relativePath)),
195+
);
196+
}
124197
} catch {
125-
// Ignore unreadable files; they remain unresolved and will be surfaced in summary text.
198+
// File unreadable from disk – still try index-based fallback.
199+
candidates.push(
200+
...(await buildFallbackCandidatesFromIndex(gitCore, cwd, relativePath)),
201+
);
126202
}
127203
}
128204
return candidates;
@@ -136,7 +212,7 @@ const makeMergeConflictResolver = Effect.gen(function* () {
136212
try: async () => {
137213
const status = await Effect.runPromise(gitCore.statusDetails(cwd));
138214
if (status.hasConflicts) {
139-
const candidates = await readCandidatesForConflicts(cwd, status.conflictedFiles);
215+
const candidates = await readCandidatesForConflicts(cwd, status.conflictedFiles, gitCore);
140216
return {
141217
status: "conflicted" as const,
142218
mergeableState: pullRequest.mergeable,
@@ -193,10 +269,12 @@ const makeMergeConflictResolver = Effect.gen(function* () {
193269
const absolutePath = path.join(cwd, candidate.path);
194270
const contents = await fsPromises.readFile(absolutePath, "utf8");
195271
const parsed = parseFirstConflictBlock(contents);
196-
if (!parsed) {
197-
throw new Error("Conflict markers were not found in the target file.");
198-
}
199-
const nextContents = `${parsed.before}${candidate.previewPatch}${parsed.after}`;
272+
// When markers are parseable, splice the candidate into the
273+
// surrounding context. Otherwise the candidate contains the
274+
// full file content (index-based fallback) – write it directly.
275+
const nextContents = parsed
276+
? `${parsed.before}${candidate.previewPatch}${parsed.after}`
277+
: candidate.previewPatch;
200278
await fsPromises.writeFile(absolutePath, nextContents, "utf8");
201279
return {
202280
candidateId,

apps/web/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
"@lexical/react": "^0.41.0",
3131
"@okcode/contracts": "workspace:*",
3232
"@okcode/shared": "workspace:*",
33-
"@pierre/diffs": "^1.1.0-beta.16",
33+
"@pierre/diffs": "1.1.8",
3434
"@tanstack/react-pacer": "^0.19.4",
3535
"@tanstack/react-query": "^5.90.0",
3636
"@tanstack/react-router": "^1.160.2",

bun.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)