fix: apply iteration 1 code-review fixes for diff-comments feature

- Preload: restore `repos.add` / `repos.addRemote` error-union return types
  so the declared preload contract matches the main-process IPC handlers
  and the renderer's `'error' in result` branching.
- Store: drop dead `filePath` param from `getDiffComments`, replace the
  home-rolled `generateId` fallback with `globalThis.crypto.randomUUID()`,
  freeze the `EMPTY_COMMENTS` sentinel, and short-circuit rollback when
  the worktree has been removed.
- UI: preserve the draft body when `addDiffComment` fails, remount
  `DiffCommentPopover` per anchor line, add dialog ARIA on the popover
  and an `aria-label` on the inline delete button, give the "+" button
  a visible focus ring, raise the popover z-index above Monaco widgets,
  set `type="button"` on the Comments trigger, and add a "Why" comment
  on the `diffComments` forward in `mergeWorktree`.

src/main/ipc/worktree-logic.ts

@@ -170,6 +170,9 @@ export function mergeWorktree(
     isPinned: meta?.isPinned ?? false,
     sortOrder: meta?.sortOrder ?? 0,
     lastActivityAt: meta?.lastActivityAt ?? 0,
+    // Why: diff comments are persisted on WorktreeMeta (see `WorktreeMeta` in
+    // shared/types) and forwarded verbatim so the renderer store mirrors
+    // on-disk state. `undefined` here means the worktree has no comments yet.
     diffComments: meta?.diffComments
   }
 }

src/preload/api-types.d.ts

@@ -269,7 +269,11 @@ export type PreloadApi = {
   }
   repos: {
     list: () => Promise<Repo[]>
-    add: (args: { path: string; kind?: 'git' | 'folder' }) => Promise<Repo>
+    // Why: error union matches the IPC handler's return shape; renderer callers branch on `'error' in result`.
+    add: (args: {
+      path: string
+      kind?: 'git' | 'folder'
+    }) => Promise<{ repo: Repo } | { error: string }>
     remove: (args: { repoId: string }) => Promise<void>
     update: (args: {
       repoId: string
@@ -281,12 +285,13 @@ export type PreloadApi = {
     pickDirectory: () => Promise<string | null>
     clone: (args: { url: string; destination: string }) => Promise<Repo>
     cloneAbort: () => Promise<void>
+    // Why: error union matches the IPC handler's return shape; renderer callers branch on `'error' in result`.
     addRemote: (args: {
       connectionId: string
       remotePath: string
       displayName?: string
       kind?: 'git' | 'folder'
-    }) => Promise<Repo>
+    }) => Promise<{ repo: Repo } | { error: string }>
     onCloneProgress: (callback: (data: { phase: string; percent: number }) => void) => () => void
     getGitUsername: (args: { repoId: string }) => Promise<string>
     getBaseRefDefault: (args: { repoId: string }) => Promise<string>

src/renderer/src/assets/main.css

@@ -907,7 +907,10 @@
   cursor: pointer;
   z-index: 5;
   opacity: 0.7;
-  transition: opacity 100ms ease, color 100ms ease, background-color 100ms ease;
+  transition:
+    opacity 100ms ease,
+    color 100ms ease,
+    background-color 100ms ease;
 }
 
 .orca-diff-comment-add-btn:hover {
@@ -916,6 +919,11 @@
   background: color-mix(in srgb, var(--primary) 12%, var(--background));
 }
 
+.orca-diff-comment-add-btn:focus-visible {
+  outline: 2px solid var(--primary);
+  outline-offset: 1px;
+}
+
 .orca-diff-comment-inline {
   width: 100%;
   padding: 4px 8px 6px;
@@ -973,7 +981,7 @@
 .orca-diff-comment-popover {
   position: absolute;
   right: 24px;
-  z-index: 20;
+  z-index: 1000;
   width: 320px;
   padding: 8px;
   border: 1px solid var(--border);

src/renderer/src/components/diff-comments/DiffCommentPopover.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from 'react'
+import { useEffect, useId, useRef, useState } from 'react'
 import { Button } from '@/components/ui/button'
 
 // Why: rendered as a DOM sibling overlay inside the editor container rather
@@ -23,6 +23,10 @@ export function DiffCommentPopover({
   const [body, setBody] = useState('')
   const textareaRef = useRef<HTMLTextAreaElement | null>(null)
   const popoverRef = useRef<HTMLDivElement | null>(null)
+  // Why: stable id per-instance so multiple popovers (should they ever coexist)
+  // don't collide on aria-labelledby references. Screen readers announce the
+  // "Line N" label as the dialog's accessible name.
+  const labelId = useId()
 
   useEffect(() => {
     textareaRef.current?.focus()
@@ -67,10 +71,15 @@ export function DiffCommentPopover({
       ref={popoverRef}
       className="orca-diff-comment-popover"
       style={{ top: `${top}px` }}
+      role="dialog"
+      aria-modal="true"
+      aria-labelledby={labelId}
       onMouseDown={(ev) => ev.stopPropagation()}
       onClick={(ev) => ev.stopPropagation()}
     >
-      <div className="orca-diff-comment-popover-label">Line {lineNumber}</div>
+      <div id={labelId} className="orca-diff-comment-popover-label">
+        Line {lineNumber}
+      </div>
       <textarea
         ref={textareaRef}
         className="orca-diff-comment-popover-textarea"

src/renderer/src/components/diff-comments/DiffCommentsTab.tsx

@@ -161,6 +161,7 @@ export function DiffCommentsTab({ activeFile }: { activeFile: OpenFile }): React
                         className="mt-0.5 rounded p-0.5 text-muted-foreground opacity-0 transition-opacity hover:text-destructive group-hover:opacity-100"
                         onClick={() => void deleteDiffComment(worktreeId, c.id)}
                         title="Delete comment"
+                        aria-label={`Delete comment on line ${c.lineNumber}`}
                       >
                         <Trash2 className="size-3.5" />
                       </button>

src/renderer/src/components/editor/CombinedDiffViewer.tsx

@@ -497,6 +497,7 @@ export default function CombinedDiffViewer({
         </span>
         <div className="flex items-center gap-2">
           <button
+            type="button"
             className="flex items-center gap-1 text-xs text-muted-foreground hover:text-foreground transition-colors"
             onClick={() => openDiffCommentsTab(file.worktreeId, file.filePath)}
             title="Open diff comments tab"

src/renderer/src/components/editor/DiffSectionItem.tsx

@@ -163,18 +163,26 @@ export function DiffSectionItem({
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [modifiedEditor, popover?.lineNumber])
 
-  const handleSubmitComment = (body: string): void => {
+  const handleSubmitComment = async (body: string): Promise<void> => {
     if (!popover) {
       return
     }
-    void addDiffComment({
+    // Why: await persistence before closing the popover. If addDiffComment
+    // resolves to null, the store rolled back the optimistic insert; keeping
+    // the popover open preserves the user's draft so they can retry instead
+    // of silently losing their text.
+    const result = await addDiffComment({
       worktreeId,
       filePath: section.path,
       lineNumber: popover.lineNumber,
       body,
       side: 'modified'
     })
-    setPopover(null)
+    if (result) {
+      setPopover(null)
+    } else {
+      console.error('Failed to add diff comment — draft preserved')
+    }
   }
 
   const lineStats = useMemo(
@@ -314,7 +322,11 @@ export function DiffSectionItem({
           }}
         >
           {popover && (
+            // Why: key by lineNumber so the popover remounts when the anchor
+            // line changes, resetting the internal draft body and textarea
+            // focus per anchor line instead of leaking state across lines.
             <DiffCommentPopover
+              key={popover.lineNumber}
               lineNumber={popover.lineNumber}
               top={popover.top}
               onCancel={() => setPopover(null)}

src/renderer/src/store/slices/diffComments.ts

@@ -4,23 +4,23 @@ import type { DiffComment, Worktree } from '../../../../shared/types'
 import { findWorktreeById, getRepoIdFromWorktreeId } from './worktree-helpers'
 
 export type DiffCommentsSlice = {
-  getDiffComments: (worktreeId: string, filePath?: string) => DiffComment[]
+  getDiffComments: (worktreeId: string) => DiffComment[]
   addDiffComment: (input: Omit<DiffComment, 'id' | 'createdAt'>) => Promise<DiffComment | null>
   deleteDiffComment: (worktreeId: string, commentId: string) => Promise<void>
   clearDiffComments: (worktreeId: string) => Promise<void>
 }
 
 function generateId(): string {
-  if (typeof crypto !== 'undefined' && 'randomUUID' in crypto) {
-    return crypto.randomUUID()
-  }
-  return `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`
+  return globalThis.crypto.randomUUID()
 }
 
 // Why: return a stable reference when no comments exist so selectors don't
 // produce a fresh `[]` on every store update. A new array identity would
 // trigger re-renders in any consumer using referential equality.
-const EMPTY_COMMENTS: DiffComment[] = []
+// Frozen + typed `readonly` so an accidental `list.push(...)` on the returned
+// value is both a runtime TypeError and a TypeScript compile error, preventing
+// the sentinel from being corrupted globally.
+const EMPTY_COMMENTS: readonly DiffComment[] = Object.freeze([])
 
 async function persist(worktreeId: string, diffComments: DiffComment[]): Promise<void> {
   await window.api.worktrees.updateMeta({
@@ -89,10 +89,17 @@ function rollback(
       return {}
     }
     const target = repoList.find((w) => w.id === worktreeId)
+    // Why: if the worktree was removed between the optimistic mutation and
+    // this rollback, there is nothing to restore. Bail out before remapping
+    // `repoList` so we don't allocate a new outer-array identity and trigger
+    // spurious subscriber notifications.
+    if (!target) {
+      return {}
+    }
     // Why: only roll back if no other mutation landed since this one. If a
     // later write already replaced the comments array with a different
     // identity, our stale `previous` would erase that newer state.
-    if (target?.diffComments !== expectedCurrent) {
+    if (target.diffComments !== expectedCurrent) {
       return {}
     }
     const nextList: Worktree[] = repoList.map((w) =>
@@ -106,13 +113,16 @@ export const createDiffCommentsSlice: StateCreator<AppState, [], [], DiffComment
   set,
   get
 ) => ({
-  getDiffComments: (worktreeId, filePath) => {
+  getDiffComments: (worktreeId) => {
     const worktree = findWorktreeById(get().worktreesByRepo, worktreeId)
-    const all = worktree?.diffComments ?? EMPTY_COMMENTS
-    if (!filePath) {
-      return all
+    if (!worktree?.diffComments) {
+      // Why: cast the frozen sentinel to the mutable `DiffComment[]` return
+      // type. The array is frozen at runtime so accidental mutation throws;
+      // the cast only hides the `readonly` marker from consumers that never
+      // mutate the list in practice.
+      return EMPTY_COMMENTS as DiffComment[]
     }
-    return all.filter((c) => c.filePath === filePath)
+    return worktree.diffComments
   },
 
   addDiffComment: async (input) => {