Address inbox PR review comments

Co-Authored-By: Paperclip <noreply@paperclip.ing>

ui/src/components/IssuesList.test.tsx

@@ -1114,8 +1114,8 @@ describe("IssuesList", () => {
         title: `Issue ${index + 1}`,
       }),
     );
-  const onLoadMoreIssues = vi.fn();
-  setDocumentScrollMetrics({ innerHeight: 2000, scrollY: 0, scrollHeight: 1000 });
+    const onLoadMoreIssues = vi.fn();
+    setDocumentScrollMetrics({ innerHeight: 2000, scrollY: 0, scrollHeight: 1000 });
 
     const { root } = renderWithQueryClient(
       <IssuesList

ui/src/lib/inbox.test.ts

@@ -673,6 +673,32 @@ describe("inbox helpers", () => {
     ).toEqual([parentIssue.id, childIssue.id]);
   });
 
+  it("stops cyclic child issue traversal when building keyboard navigation", () => {
+    const parentIssue = makeIssue("parent", true);
+    const childIssue = makeIssue("child", true);
+    childIssue.parentId = parentIssue.id;
+    parentIssue.parentId = childIssue.id;
+
+    const groupedSections = [
+      {
+        key: "workspace:default",
+        displayItems: [{ kind: "issue", timestamp: 2, issue: parentIssue } satisfies InboxWorkItem],
+        childrenByIssueId: new Map([
+          [parentIssue.id, [childIssue]],
+          [childIssue.id, [parentIssue]],
+        ]),
+      },
+    ];
+
+    expect(
+      buildInboxKeyboardNavEntries(groupedSections, new Set(), new Set()).map((entry) => entry.type === "top"
+        ? entry.item.kind === "issue" ? entry.item.issue.id : "other"
+        : entry.type === "child"
+          ? entry.issueId
+          : entry.groupKey),
+    ).toEqual([parentIssue.id, childIssue.id]);
+  });
+
   it("emits a group nav entry for labeled groups and omits children when the group is collapsed", () => {
     const visibleIssue = makeIssue("visible", true);
     const hiddenIssue = makeIssue("hidden", true);

ui/src/lib/inbox.ts

@@ -1002,17 +1002,20 @@ export function buildInboxKeyboardNavEntries(
     }
     if (isCollapsed) continue;
 
-    const addIssueChildren = (issueId: string) => {
+    const addIssueChildren = (issueId: string, seen: ReadonlySet<string>) => {
       const children = group.childrenByIssueId.get(issueId);
       if (!children?.length || collapsedInboxParents.has(issueId)) return;
 
       for (const child of children) {
+        if (seen.has(child.id)) continue;
+        const nextSeen = new Set(seen);
+        nextSeen.add(child.id);
         entries.push({
           type: "child",
           issueId: child.id,
           issue: child,
         });
-        addIssueChildren(child.id);
+        addIssueChildren(child.id, nextSeen);
       }
     };
 
@@ -1024,7 +1027,7 @@ export function buildInboxKeyboardNavEntries(
       });
 
       if (item.kind !== "issue") continue;
-      addIssueChildren(item.issue.id);
+      addIssueChildren(item.issue.id, new Set([item.issue.id]));
     }
   }
 

ui/src/pages/Inbox.tsx

@@ -2436,8 +2436,15 @@ export function Inbox() {
                     const hasChildren = childIssues.length > 0;
                     const isExpanded = hasChildren && !collapsedInboxParents.has(issue.id);
                     const canArchiveIssue = canArchiveFromTab && group.searchSection === "none";
-                    const renderChildIssueRows = (children: Issue[], depth: number): ReactNode[] =>
+                    const renderChildIssueRows = (
+                      children: Issue[],
+                      depth: number,
+                      seen: ReadonlySet<string>,
+                    ): ReactNode[] =>
                       children.flatMap((child) => {
+                        if (seen.has(child.id)) return [];
+                        const nextSeen = new Set(seen);
+                        nextSeen.add(child.id);
                         const childNavIdx = childFlatIndex.get(child.id) ?? -1;
                         const isChildSelected = selectedIndex === childNavIdx;
                         const grandchildIssues = group.childrenByIssueId.get(child.id) ?? [];
@@ -2475,7 +2482,7 @@ export function Inbox() {
                         );
 
                         return childIsExpanded
-                          ? [row, ...renderChildIssueRows(grandchildIssues, depth + 1)]
+                          ? [row, ...renderChildIssueRows(grandchildIssues, depth + 1, nextSeen)]
                           : [row];
                       });
                     const parentRow = renderInboxIssue({
@@ -2501,7 +2508,7 @@ export function Inbox() {
                     ) : parentRow));
 
                     if (isExpanded) {
-                      elements.push(...renderChildIssueRows(childIssues, 1));
+                      elements.push(...renderChildIssueRows(childIssues, 1, new Set([issue.id])));
                     }
                   }