fix(comment): address review feedback on pin-follow-scroll

- Fix first-mount race: WATCH_SELECTORS could be posted before the iframe
  overlay's message listener had installed. Thread an onIframeLoaded
  callback up from PreviewSlot into a load-tick state the effect depends
  on, so switching back to a design with existing pins now re-registers
  watchers and the pins track scroll from the first frame.
- Clamp ELEMENT_RECTS.entries to MAX_ELEMENT_RECTS_ENTRIES (256) in the
  validator so hostile LLM HTML can't flood the parent's liveRects map
  by spoofing the message envelope.
- Replace three inline zoom-scaling blocks with scaleRectForZoom.
- Delete the legacy pinStyle(comment, zoom) wrapper (prod now uses
  pinStyleFromRect directly); migrate its tests to the new signature.

Author: hqhq1025

apps/desktop/src/renderer/src/components/PreviewPane.test.ts

@@ -149,6 +149,22 @@ describe('handlePreviewMessage trust boundary', () => {
     expect(outcome.status).toBe('rejected');
     expect(handlers.onElementRects).not.toHaveBeenCalled();
   });
+
+  it('rejects ELEMENT_RECTS whose entries array exceeds the hard cap', () => {
+    // An LLM-controlled iframe script could try to flood liveRects. Validator
+    // should drop the message before it reaches the handler.
+    const handlers = makeHandlers();
+    const entries = Array.from({ length: 257 }, (_, i) => ({
+      selector: `#a${i}`,
+      rect: { top: 0, left: 0, width: 1, height: 1 },
+    }));
+    const outcome = handlePreviewMessage(
+      { __codesign: true, type: 'ELEMENT_RECTS', entries },
+      handlers,
+    );
+    expect(outcome.status).toBe('rejected');
+    expect(handlers.onElementRects).not.toHaveBeenCalled();
+  });
 });
 
 describe('postModeToPreviewWindow', () => {

apps/desktop/src/renderer/src/components/PreviewPane.tsx

@@ -8,7 +8,7 @@ import {
   isIframeErrorMessage,
   isOverlayMessage,
 } from '@open-codesign/runtime';
-import { useCallback, useEffect, useMemo, useRef } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { EmptyState } from '../preview/EmptyState';
 import { ErrorState } from '../preview/ErrorState';
 import { useCodesignStore } from '../store';
@@ -134,6 +134,7 @@ interface PreviewSlotProps {
   interactionMode: string;
   registerIframe: (designId: string, el: HTMLIFrameElement | null) => void;
   onIframeError: (message: string) => void;
+  onIframeLoaded: (designId: string) => void;
 }
 
 // One iframe per pool entry. Hidden (display:none) when not active, but kept
@@ -153,6 +154,7 @@ function PreviewSlot({
   interactionMode,
   registerIframe,
   onIframeError,
+  onIframeLoaded,
 }: PreviewSlotProps) {
   const srcDocStableKey = useMemo(() => {
     return html
@@ -193,6 +195,10 @@ function PreviewSlot({
         if (!active) return;
         const target = e.currentTarget as HTMLIFrameElement;
         postModeToPreviewWindow(target.contentWindow, interactionMode, onIframeError);
+        // The parent's WATCH_SELECTORS post can race past a freshly-mounted
+        // iframe before its message listener installs. Ping the parent so it
+        // re-broadcasts after load has confirmed the overlay is live.
+        onIframeLoaded(designId);
       }}
       className={
         isMobile
@@ -294,6 +300,10 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
   // or the active iframe element re-mounts.
   const iframeRef = useRef<HTMLIFrameElement | null>(null);
   const iframesByDesign = useRef<Map<string, HTMLIFrameElement>>(new Map());
+  // Bumped every time the active iframe fires onLoad — used to re-trigger
+  // the WATCH_SELECTORS effect so we don't race past overlay installation
+  // on first mount.
+  const [iframeLoadTick, setIframeLoadTick] = useState(0);
 
   const registerIframe = useCallback((designId: string, el: HTMLIFrameElement | null) => {
     if (el) {
@@ -303,6 +313,13 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
     }
   }, []);
 
+  const handleIframeLoaded = useCallback(
+    (designId: string) => {
+      if (designId === currentDesignId) setIframeLoadTick((t) => t + 1);
+    },
+    [currentDesignId],
+  );
+
   // When the active design changes, retarget iframeRef and re-broadcast the
   // current interaction mode. Background iframes keep their last mode — fine,
   // they're inert until reactivated.
@@ -325,7 +342,7 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
   // Selectors: all comments on the current snapshot + the active bubble's
   // selector (usually the freshly-pinned one, included for the moment
   // between click and save).
-  // biome-ignore lint/correctness/useExhaustiveDependencies: currentDesignId is intentional — iframeRef.current is a ref so biome can't see that it changes with the active design. When the design switches we MUST resend WATCH_SELECTORS to the newly-active iframe; the currentDesignId dependency is what triggers that re-run.
+  // biome-ignore lint/correctness/useExhaustiveDependencies: currentDesignId and iframeLoadTick are deliberate triggers — iframeRef.current is a ref so biome can't see it swap when the active design changes, and we must wait for the iframe's onLoad before the overlay's message listener exists (otherwise the post is dropped).
   useEffect(() => {
     const win = iframeRef.current?.contentWindow;
     if (!win) return;
@@ -344,7 +361,7 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
     } catch {
       /* sandbox gone — retry happens next render */
     }
-  }, [comments, currentSnapshotId, commentBubble, currentDesignId]);
+  }, [comments, currentSnapshotId, commentBubble, currentDesignId, iframeLoadTick]);
 
   useEffect(() => {
     function onMessage(event: MessageEvent): void {
@@ -429,12 +446,7 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
           selector: c.selector,
           tag: c.tag,
           outerHTML: c.outerHTML,
-          rect: {
-            top: live.top * (previewZoom / 100),
-            left: live.left * (previewZoom / 100),
-            width: live.width * (previewZoom / 100),
-            height: live.height * (previewZoom / 100),
-          },
+          rect: scaleRectForZoom(live, previewZoom),
           existingCommentId: c.id,
           initialText: c.text,
         });
@@ -494,6 +506,7 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
             interactionMode={interactionMode}
             registerIframe={registerIframe}
             onIframeError={pushIframeError}
+            onIframeLoaded={handleIframeLoaded}
           />
         ))}
         {!activeHasHtml ? (
@@ -530,12 +543,7 @@ export function PreviewPane({ onPickStarter }: PreviewPaneProps) {
           ? (() => {
               const liveForBubble = liveRects[commentBubble.selector];
               const scaled = liveForBubble
-                ? {
-                    top: liveForBubble.top * (previewZoom / 100),
-                    left: liveForBubble.left * (previewZoom / 100),
-                    width: liveForBubble.width * (previewZoom / 100),
-                    height: liveForBubble.height * (previewZoom / 100),
-                  }
+                ? scaleRectForZoom(liveForBubble, previewZoom)
                 : commentBubble.rect;
               return (
                 <CommentBubble

apps/desktop/src/renderer/src/components/comment/PinOverlay.test.tsx

@@ -1,6 +1,6 @@
 import type { CommentRow } from '@open-codesign/shared';
 import { describe, expect, it } from 'vitest';
-import { pinStyle, pinStyleFromRect, variantFor } from './PinOverlay';
+import { pinStyleFromRect, variantFor } from './PinOverlay';
 
 function comment(overrides: Partial<CommentRow> = {}): CommentRow {
   return {
@@ -38,23 +38,22 @@ describe('variantFor', () => {
   });
 });
 
-describe('pinStyle', () => {
+describe('pinStyleFromRect', () => {
   it('anchors to the top-right corner of the rect, offset by 10px', () => {
-    const pos = pinStyle(comment({ rect: { top: 100, left: 50, width: 80, height: 40 } }), 100);
+    const pos = pinStyleFromRect({ top: 100, left: 50, width: 80, height: 40 }, 100);
     // top = 100 - 10 = 90; left = 50 + 80 - 10 = 120
     expect(pos).toEqual({ top: '90px', left: '120px' });
   });
 
   it('scales with zoom', () => {
-    const pos = pinStyle(comment({ rect: { top: 100, left: 50, width: 80, height: 40 } }), 50);
+    const pos = pinStyleFromRect({ top: 100, left: 50, width: 80, height: 40 }, 50);
     // scale = 0.5; top = 100*0.5 - 10 = 40; left = 50*0.5 + 80*0.5 - 10 = 55
     expect(pos).toEqual({ top: '40px', left: '55px' });
   });
 
-  it('pinStyleFromRect uses the supplied rect instead of the stored one', () => {
-    // This is the path PinOverlay takes when liveRects has an entry for the
-    // comment's selector: the badge stays glued to the element's current
-    // position in the iframe viewport, even after the user scrolls.
+  it('takes a rect argument so the overlay can substitute a live rect', () => {
+    // PinOverlay calls this with liveRects[selector] ?? stored — what keeps
+    // the badge glued to the element after iframe scroll.
     const live = pinStyleFromRect({ top: 10, left: 5, width: 80, height: 40 }, 100);
     expect(live).toEqual({ top: '0px', left: '75px' });
   });

apps/desktop/src/renderer/src/components/comment/PinOverlay.tsx

@@ -61,10 +61,6 @@ export function pinStyleFromRect(rect: CommentRect, zoom: number): { top: string
   return { top: `${top}px`, left: `${left}px` };
 }
 
-export function pinStyle(comment: CommentRow, zoom: number): { top: string; left: string } {
-  return pinStyleFromRect(comment.rect, zoom);
-}
-
 export function PinOverlay({ comments, zoom, onPinClick, liveRects }: PinOverlayProps) {
   const t = useT();
   if (comments.length === 0) return null;

packages/runtime/src/overlay.ts

@@ -334,11 +334,19 @@ export interface ElementRectsMessage {
   }>;
 }
 
+/** Hard ceiling on entries per ELEMENT_RECTS message. The iframe runs LLM
+ *  HTML; even though our overlay is trusted, untrusted in-iframe code can
+ *  synthesise a matching envelope. Cap worst-case memory growth in the
+ *  parent's liveRects store. Chosen generously — a design with 256 tracked
+ *  pins is already beyond any realistic review session. */
+export const MAX_ELEMENT_RECTS_ENTRIES = 256;
+
 export function isElementRectsMessage(data: unknown): data is ElementRectsMessage {
   if (typeof data !== 'object' || data === null) return false;
   const d = data as { __codesign?: boolean; type?: string; entries?: unknown };
   if (d.__codesign !== true || d.type !== 'ELEMENT_RECTS') return false;
   if (!Array.isArray(d.entries)) return false;
+  if (d.entries.length > MAX_ELEMENT_RECTS_ENTRIES) return false;
   for (const e of d.entries) {
     if (typeof e !== 'object' || e === null) return false;
     const entry = e as { selector?: unknown; rect?: unknown };