Hide visible merge conflict markers in PR review

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

@@ -3,7 +3,7 @@ import { promises as fsPromises } from "node:fs";
 
 import { Effect, Layer } from "effect";
 import type { PrConflictCandidateResolution, PrReviewSummary } from "@okcode/contracts";
-import { GitCore } from "../../git/Services/GitCore.ts";
+import { GitCore, type GitCoreShape } from "../../git/Services/GitCore.ts";
 import {
   MergeConflictResolver,
   type MergeConflictResolverShape,
@@ -114,15 +114,91 @@ function buildCandidatesForFile(input: {
   return candidates;
 }
 
-async function readCandidatesForConflicts(cwd: string, conflictedFiles: readonly string[]) {
+/**
+ * Read the "ours" (stage 2) and "theirs" (stage 3) versions of a conflicted
+ * file directly from the git index.  This works even when the working-tree
+ * copy has no parseable conflict markers (e.g. binary files, diff3 style,
+ * already-partially-resolved markers, or multiple conflict blocks).
+ */
+async function buildFallbackCandidatesFromIndex(
+  gitCore: GitCoreShape,
+  cwd: string,
+  relativePath: string,
+): Promise<PrConflictCandidateResolution[]> {
+  const candidates: PrConflictCandidateResolution[] = [];
+  const tryStage = async (
+    stage: "2" | "3",
+    label: "ours" | "theirs",
+    title: string,
+    description: string,
+  ) => {
+    try {
+      const result = await Effect.runPromise(
+        gitCore.execute({
+          operation: "showConflictStage",
+          cwd,
+          args: ["show", `:${stage}:${relativePath}`],
+          allowNonZeroExit: true,
+        }),
+      );
+      if (result.code === 0) {
+        candidates.push(
+          buildCandidate({
+            id: `${relativePath}:${label}`,
+            path: relativePath,
+            title,
+            description,
+            confidence: "review",
+            replacement: result.stdout,
+          }),
+        );
+      }
+    } catch {
+      // Stage does not exist in the index; skip this side.
+    }
+  };
+
+  await tryStage(
+    "2",
+    "ours",
+    "Prefer current side (full file)",
+    "Review-required candidate using the full current-branch version from the git index.",
+  );
+  await tryStage(
+    "3",
+    "theirs",
+    "Prefer incoming side (full file)",
+    "Review-required candidate using the full incoming-branch version from the git index.",
+  );
+
+  return candidates;
+}
+
+async function readCandidatesForConflicts(
+  cwd: string,
+  conflictedFiles: readonly string[],
+  gitCore: GitCoreShape,
+) {
   const candidates: PrConflictCandidateResolution[] = [];
   for (const relativePath of conflictedFiles) {
     try {
       const absolutePath = path.join(cwd, relativePath);
       const contents = await fsPromises.readFile(absolutePath, "utf8");
-      candidates.push(...buildCandidatesForFile({ relativePath, contents }));
+      const fileCandidates = buildCandidatesForFile({ relativePath, contents });
+      if (fileCandidates.length > 0) {
+        candidates.push(...fileCandidates);
+      } else {
+        // Marker parsing failed (diff3 style, multiple blocks, etc.) – fall
+        // back to full-file ours/theirs from the git index.
+        candidates.push(
+          ...(await buildFallbackCandidatesFromIndex(gitCore, cwd, relativePath)),
+        );
+      }
     } catch {
-      // Ignore unreadable files; they remain unresolved and will be surfaced in summary text.
+      // File unreadable from disk – still try index-based fallback.
+      candidates.push(
+        ...(await buildFallbackCandidatesFromIndex(gitCore, cwd, relativePath)),
+      );
     }
   }
   return candidates;
@@ -136,7 +212,7 @@ const makeMergeConflictResolver = Effect.gen(function* () {
       try: async () => {
         const status = await Effect.runPromise(gitCore.statusDetails(cwd));
         if (status.hasConflicts) {
-          const candidates = await readCandidatesForConflicts(cwd, status.conflictedFiles);
+          const candidates = await readCandidatesForConflicts(cwd, status.conflictedFiles, gitCore);
           return {
             status: "conflicted" as const,
             mergeableState: pullRequest.mergeable,
@@ -193,10 +269,12 @@ const makeMergeConflictResolver = Effect.gen(function* () {
               const absolutePath = path.join(cwd, candidate.path);
               const contents = await fsPromises.readFile(absolutePath, "utf8");
               const parsed = parseFirstConflictBlock(contents);
-              if (!parsed) {
-                throw new Error("Conflict markers were not found in the target file.");
-              }
-              const nextContents = `${parsed.before}${candidate.previewPatch}${parsed.after}`;
+              // When markers are parseable, splice the candidate into the
+              // surrounding context.  Otherwise the candidate contains the
+              // full file content (index-based fallback) – write it directly.
+              const nextContents = parsed
+                ? `${parsed.before}${candidate.previewPatch}${parsed.after}`
+                : candidate.previewPatch;
               await fsPromises.writeFile(absolutePath, nextContents, "utf8");
               return {
                 candidateId,

apps/web/package.json

@@ -30,7 +30,7 @@
     "@lexical/react": "^0.41.0",
     "@okcode/contracts": "workspace:*",
     "@okcode/shared": "workspace:*",
-    "@pierre/diffs": "^1.1.0-beta.16",
+    "@pierre/diffs": "1.1.8",
     "@tanstack/react-pacer": "^0.19.4",
     "@tanstack/react-query": "^5.90.0",
     "@tanstack/react-router": "^1.160.2",

apps/web/src/components/skills/SkillsPage.tsx

@@ -138,30 +138,53 @@ function SkillDetailDialog(props: {
   const mutable = isCatalog ? !skill.immutable && skill.installed : skill.mutable;
   const pathValue = skill.path;
   const slashName = isCatalog ? skill.name.toLowerCase().replace(/\s+/g, "-") : skill.name;
+  const tags = "tags" in skill ? skill.tags : [];
+  const Icon = skillIcon("icon" in skill ? skill.icon : "file");
   return (
     <Dialog open={props.open} onOpenChange={props.onOpenChange}>
       <DialogPopup>
         <DialogHeader>
-          <DialogTitle>{skill.name}</DialogTitle>
-          <DialogDescription>{skill.description}</DialogDescription>
-        </DialogHeader>
-        <DialogPanel className="space-y-4">
-          <div className="flex flex-wrap gap-2">
-            {("tags" in skill ? skill.tags : []).map((tag) => (
-              <Badge key={tag} variant="secondary">
-                {tag}
-              </Badge>
-            ))}
+          <div className="flex items-center gap-3">
+            <div className="flex size-10 shrink-0 items-center justify-center rounded-xl bg-accent/70 text-foreground">
+              <Icon className="size-5" />
+            </div>
+            <div className="min-w-0">
+              <DialogTitle>{skill.name}</DialogTitle>
+              <DialogDescription className="mt-1">{skill.description}</DialogDescription>
+            </div>
           </div>
+          {tags.length > 0 && (
+            <div className="flex flex-wrap gap-1.5 pt-1">
+              {tags.map((tag) => (
+                <Badge key={tag} variant="secondary" className="text-xs">
+                  {tag}
+                </Badge>
+              ))}
+            </div>
+          )}
+        </DialogHeader>
+        <DialogPanel className="space-y-3">
           <div className="rounded-xl border bg-muted/35 p-3 text-sm">
-            <p className="font-medium text-foreground">Slash commands</p>
-            <p className="mt-2 font-mono text-xs text-muted-foreground">/{slashName}</p>
-            <p className="mt-1 font-mono text-xs text-muted-foreground">/skill read {slashName}</p>
+            <p className="font-medium text-foreground text-xs uppercase tracking-wider text-muted-foreground">
+              Slash commands
+            </p>
+            <div className="mt-2 space-y-1">
+              <p className="rounded-md bg-muted/50 px-2 py-1 font-mono text-xs text-foreground">
+                /{slashName}
+              </p>
+              <p className="rounded-md bg-muted/50 px-2 py-1 font-mono text-xs text-foreground">
+                /skill read {slashName}
+              </p>
+            </div>
           </div>
           {pathValue ? (
             <div className="rounded-xl border bg-muted/35 p-3 text-sm">
-              <p className="font-medium text-foreground">Path</p>
-              <p className="mt-1 break-all font-mono text-xs text-muted-foreground">{pathValue}</p>
+              <p className="font-medium text-xs uppercase tracking-wider text-muted-foreground">
+                Installed path
+              </p>
+              <p className="mt-2 break-all rounded-md bg-muted/50 px-2 py-1 font-mono text-xs text-foreground">
+                {pathValue}
+              </p>
             </div>
           ) : null}
         </DialogPanel>
@@ -422,7 +445,7 @@ export function SkillsPage(props: {
             <SkillLibraryTabs current="skills" />
           </div>
         </div>
-        <div className="mx-auto flex w-full max-w-6xl flex-1 flex-col gap-8 px-4 py-8 sm:px-6">
+        <div className="mx-auto flex w-full max-w-6xl flex-1 flex-col gap-8 overflow-y-auto px-4 py-8 sm:px-6">
           <div className="space-y-6">
             <div className="flex flex-col gap-4 lg:flex-row lg:items-center lg:justify-between">
               <div className="space-y-2">

apps/web/src/components/ui/dialog.tsx

@@ -90,7 +90,13 @@ function DialogHeader({ className, ...props }: React.ComponentProps<"div">) {
 }
 
 function DialogPanel({ className, ...props }: React.ComponentProps<"div">) {
-  return <div className={cn("px-6 pb-6", className)} data-slot="dialog-panel" {...props} />;
+  return (
+    <div
+      className={cn("min-h-0 overflow-y-auto px-6 pb-6", className)}
+      data-slot="dialog-panel"
+      {...props}
+    />
+  );
 }
 
 function DialogFooter({