fix(inbox): re-fire scroll-to-comment effect once issue finishes loading

When clicking an inbox notification for a different issue, the IssueDetail
remounts and both the issue detail and timeline queries fetch in parallel.
If the timeline query resolves first, `timeline.length` flips to >0 while
`loading` is still true — at that moment the component is rendering the
skeleton, so `getElementById('comment-<id>')` returns null and the scroll
silently fails. Without `loading` in the effect's deps, the effect never
re-runs when the issue finally loads, leaving the user at the top of the
issue instead of jumping to the highlighted comment.

Add `loading` to the early-return guard and to the dep list so the scroll
fires once both the issue and its comments are mounted. The dropped
`return () => clearTimeout(timer)` was inside requestAnimationFrame and
never functioned as cleanup — removed for clarity.

Test seeds the timeline cache and holds back the issue fetch to reproduce
the race deterministically; without the fix the regression test times out
waiting for scrollIntoView.

Co-authored-by: multica-agent <github@multica.ai>

packages/views/issues/components/issue-detail.test.tsx

@@ -375,6 +375,30 @@ function renderIssueDetail(issueId = "issue-1") {
   );
 }
 
+function renderIssueDetailWithHighlight(
+  highlightCommentId: string,
+  issueId = "issue-1",
+  options: { seedTimeline?: boolean } = {},
+) {
+  const queryClient = createTestQueryClient();
+  if (options.seedTimeline) {
+    // Pre-populate the timeline cache so the first render sees timeline.length>0.
+    // This reproduces the inbox-click race: timeline data is available before
+    // the issue itself has finished loading, so the effect that scrolls to
+    // the comment fires once with `loading=true` (skeleton still rendered,
+    // no comment DOM) and must re-fire when `loading` flips to false.
+    queryClient.setQueryData(["issues", "timeline", issueId], mockTimeline);
+  }
+  const result = render(
+    <I18nProvider locale="en" resources={TEST_RESOURCES}>
+      <QueryClientProvider client={queryClient}>
+        <IssueDetail issueId={issueId} highlightCommentId={highlightCommentId} />
+      </QueryClientProvider>
+    </I18nProvider>,
+  );
+  return { ...result, queryClient };
+}
+
 // ---------------------------------------------------------------------------
 // Tests
 // ---------------------------------------------------------------------------
@@ -510,6 +534,67 @@ describe("IssueDetail (shared)", () => {
     expect(screen.getByText("I can help with this")).toBeInTheDocument();
   });
 
+  describe("highlightCommentId scroll-to-comment", () => {
+    let scrollIntoViewSpy: ReturnType<typeof vi.fn>;
+
+    beforeEach(() => {
+      scrollIntoViewSpy = vi.fn();
+      Element.prototype.scrollIntoView =
+        scrollIntoViewSpy as unknown as Element["scrollIntoView"];
+    });
+
+    it("scrolls to the highlighted comment after both issue and timeline finish loading", async () => {
+      renderIssueDetailWithHighlight("comment-2");
+
+      // Wait until the comment DOM is rendered.
+      await waitFor(() => {
+        expect(document.getElementById("comment-comment-2")).not.toBeNull();
+      });
+
+      // requestAnimationFrame defers the actual scrollIntoView call.
+      await waitFor(() => {
+        expect(scrollIntoViewSpy).toHaveBeenCalled();
+      });
+
+      const callContext = scrollIntoViewSpy.mock.contexts[0] as HTMLElement;
+      expect(callContext.id).toBe("comment-comment-2");
+    });
+
+    it("still scrolls when the timeline is ready before the issue (regression for inbox click)", async () => {
+      // Reproduces the inbox-click race: timeline data is already in the cache
+      // (resolved first), but the issue is still pending — so the first render
+      // sees timeline.length=2 alongside loading=true (skeleton still showing,
+      // no comment DOM). The scroll effect fires once, fails to find the
+      // element, and must re-fire when `loading` flips to false. Without
+      // `loading` in the dep list, that second fire never happens and the
+      // user lands at the top of the issue.
+      let resolveIssue: (value: Issue) => void = () => {};
+      const issuePromise = new Promise<Issue>((resolve) => {
+        resolveIssue = resolve;
+      });
+      mockApiObj.getIssue.mockReturnValue(issuePromise);
+
+      renderIssueDetailWithHighlight("comment-2", "issue-1", { seedTimeline: true });
+
+      // The skeleton is still showing (issue pending), so even though
+      // timeline.length>0 the comment DOM is not mounted and no scroll
+      // can happen yet.
+      expect(document.getElementById("comment-comment-2")).toBeNull();
+      expect(scrollIntoViewSpy).not.toHaveBeenCalled();
+
+      // Now the issue resolves — comment elements mount, the effect re-runs
+      // because `loading` is part of its deps, and the scroll fires.
+      resolveIssue(mockIssue);
+
+      await waitFor(() => {
+        expect(document.getElementById("comment-comment-2")).not.toBeNull();
+      });
+      await waitFor(() => {
+        expect(scrollIntoViewSpy).toHaveBeenCalled();
+      });
+    });
+  });
+
   it("sends empty description when editor is cleared", async () => {
     renderIssueDetail();
 

packages/views/issues/components/issue-detail.tsx

@@ -451,9 +451,15 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
 
   const loading = issueLoading;
 
-  // Scroll to highlighted comment once timeline loads (fire only once per highlightCommentId)
+  // Scroll to highlighted comment once both the issue and its timeline are
+  // available (fire only once per highlightCommentId). `loading` must be in
+  // the dep list: when timeline.length flips to >0 while the issue itself is
+  // still loading, the component is still rendering the skeleton, so
+  // getElementById finds nothing — without re-running on the loading→false
+  // transition, the scroll silently never happens and the user lands at the
+  // top of the issue.
   useEffect(() => {
-    if (!highlightCommentId || timeline.length === 0) return;
+    if (!highlightCommentId || timeline.length === 0 || loading) return;
     if (didHighlightRef.current === highlightCommentId) return;
     const el = document.getElementById(`comment-${highlightCommentId}`);
     if (el) {
@@ -461,11 +467,10 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
       requestAnimationFrame(() => {
         el.scrollIntoView({ behavior: "instant", block: "center" });
         setHighlightedId(highlightCommentId);
-        const timer = setTimeout(() => setHighlightedId(null), 2000);
-        return () => clearTimeout(timer);
+        setTimeout(() => setHighlightedId(null), 2000);
       });
     }
-  }, [highlightCommentId, timeline.length]);
+  }, [highlightCommentId, timeline.length, loading]);
 
   const descEditorRef = useRef<ContentEditorRef>(null);
   const { isDragOver: descDragOver, dropZoneProps: descDropZoneProps } = useFileDropZone({