fix(browser): refuse a 21st page annotation with a visible limit instead of dropping the oldest (#35979)

* fix(browser): refuse a 21st page annotation with a visible limit message instead of dropping the oldest The store evicted the oldest note once a page held 20, so feedback vanished unseen. It now refuses the add; picking an element on a full page shows a limit toast instead of opening a draft, and the annotate banner states the limit. * fix(browser): name the actions that free an annotation slot * fix(browser): skip the limit toast for a selection loading already cancelled * refactor(browser): one isBrowserAnnotationPageFull rule for the store, hook and banner * refactor(browser): move the annotation limit message out of the grab hook * fix(i18n): name the annotation limit placeholder
2026-10-31 17:17:38 +00:00 · 2026-10-29 20:48:13 +00:00
parent 93d8ce7767
commit 0cdb7b35cd
19 changed files with 123 additions and 70 deletions
@@ -0,0 +1,9 @@
+import { translate } from '@/i18n/i18n'
+import { GRAB_BUDGET } from '../../../../../shared/browser-grab-types'
+
+export const browserAnnotationLimitMessage = (): string =>
+  translate(
+    'auto.components.browser.pane.annotate.use.browser.page.grab.annotations.limitReached',
+    'Limit of {{limit}} annotations reached. Send, delete or clear some to add more.',
+    { limit: GRAB_BUDGET.annotationsMaxPerPage }
+  )
@@ -4,9 +4,10 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { createTestStore } from '@/store/slices/browser-slice-test-harness'
 import { makeAnnotation } from '@/store/slices/browser-annotation-test-fixture'
-import type {
-  BrowserGrabResult,
-  BrowserCaptureSelectionScreenshotResult
+import {
+  GRAB_BUDGET,
+  type BrowserGrabResult,
+  type BrowserCaptureSelectionScreenshotResult
 } from '../../../../../shared/browser-grab-types'
 import { useGrabMode } from './useGrabMode'
 import { useBrowserPageGrabAnnotations } from './use-browser-page-grab-annotations'
@@ -40,7 +41,7 @@ afterEach(() => {
  Reflect.deleteProperty(window, 'api')
 })

-function mount(invalidateBeforePendingEffect = false) {
+function mount(invalidateBeforePendingEffect = false, browserAnnotationsLength = 0) {
  const selection = deferred<BrowserGrabResult>()
  const screenshot = deferred<BrowserCaptureSelectionScreenshotResult>()
  const awaitGrabSelection = vi
@@ -72,7 +73,7 @@ function mount(invalidateBeforePendingEffect = false) {
      containerRef,
      webviewRef,
      setBrowserOverlayViewport,
-      browserAnnotationsLength: 0
+      browserAnnotationsLength
    })
    const { cancelPendingBrowserCapture } = annotations
    useLayoutEffect(() => {
@@ -187,3 +188,31 @@ describe('capture cancellation on a document boundary', () => {
    ])
  })
 })
+
+describe('per-page annotation limit', () => {
+  it('tells the user the page is full instead of opening a draft that cannot be saved', async () => {
+    const h = mount(false, GRAB_BUDGET.annotationsMaxPerPage)
+    act(() => h.result.current.annotations.startGrabIntent('annotate'))
+    await waitFor(() => expect(h.awaitGrabSelection).toHaveBeenCalledOnce())
+    await act(async () => {
+      h.selection.resolve(selected())
+      h.screenshot.resolve({ ok: false, reason: 'fixture' })
+      await h.screenshot.promise
+    })
+    await waitFor(() => expect(h.result.current.annotations.grabToast?.type).toBe('error'))
+    expect(h.result.current.annotations.pendingAnnotationPayload).toBeNull()
+  })
+
+  it('shows no limit toast for a selection that loading cancelled first', async () => {
+    const h = mount(true, GRAB_BUDGET.annotationsMaxPerPage)
+    act(() => h.result.current.annotations.startGrabIntent('annotate'))
+    await waitFor(() => expect(h.awaitGrabSelection).toHaveBeenCalledOnce())
+    await act(async () => {
+      h.selection.resolve(selected())
+      h.screenshot.resolve({ ok: false, reason: 'fixture' })
+      await h.screenshot.promise
+    })
+    expect(h.result.current.grab.state).toBe('idle')
+    expect(h.result.current.annotations.grabToast).toBeNull()
+  })
+})
@@ -11,11 +11,13 @@ import {
 import { translate } from '@/i18n/i18n'
 import { useMountedRef } from '@/hooks/useMountedRef'
 import { useAppStore } from '@/store'
-import type {
-  BrowserAnnotationIntent,
-  BrowserGrabPayload
+import {
+  isBrowserAnnotationPageFull,
+  type BrowserAnnotationIntent,
+  type BrowserGrabPayload
 } from '../../../../../shared/browser-grab-types'
 import { formatGrabPayloadAsText } from './GrabConfirmationSheet'
+import { browserAnnotationLimitMessage } from './browser-annotation-limit-message'
 import {
  createBrowserAnnotationId,
  createBrowserAnnotationPayload,
@@ -85,14 +87,16 @@ export function useBrowserPageGrabAnnotations({
  const [grabToast, setGrabToast] = useState<BrowserPageGrabToastState | null>(null)
  const grabRef = useRef(grab)
  const grabPayloadRef = useRef(grab.payload)
+  const browserAnnotationsLengthRef = useRef(browserAnnotationsLength)

  useLayoutEffect(() => {
+    browserAnnotationsLengthRef.current = browserAnnotationsLength
    toolTargetIdRef.current = toolTargetId
    grabIntentRef.current = grabIntent
    pendingAnnotationPayloadRef.current = pendingAnnotationPayload
    grabRef.current = grab
    grabPayloadRef.current = grab.payload
-  }, [grab, grabIntent, pendingAnnotationPayload, toolTargetId])
+  }, [browserAnnotationsLength, grab, grabIntent, pendingAnnotationPayload, toolTargetId])
  // Why: Radix fires onOpenChange(false) before onSelect, so this flag lets onOpenChange skip the rearm that would clear the payload first.
  const grabMenuActionTakenRef = useRef(false)
  const recordFeatureInteraction = useAppStore((s) => s.recordFeatureInteraction)
@@ -150,6 +154,14 @@ export function useBrowserPageGrabAnnotations({
      return
    }
    if (grabIntent === 'annotate') {
+      // The store refuses notes past the cap, so say so before the user writes one.
+      if (
+        grabPayloadRef.current === grab.payload &&
+        isBrowserAnnotationPageFull(browserAnnotationsLengthRef.current)
+      ) {
+        showGrabToast(browserAnnotationLimitMessage(), 'error', grab.payload)
+        return
+      }
      // Loading may cancel the selection before React accepts this pending snapshot.
      setPendingAnnotationPayload(() =>
        grabPayloadRef.current === grab.payload ? grab.payload : null
@@ -10,7 +10,11 @@ import {
 import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
 import { translate } from '@/i18n/i18n'
 import { BrowserAnnotationSendMenuContent } from '../annotate/BrowserAnnotationSendMenuContent'
-import type { BrowserGrabPayload } from '../../../../../shared/browser-grab-types'
+import { browserAnnotationLimitMessage } from '../annotate/browser-annotation-limit-message'
+import {
+  isBrowserAnnotationPageFull,
+  type BrowserGrabPayload
+} from '../../../../../shared/browser-grab-types'
 import type { GrabModeHook } from '../annotate/useGrabMode'
 import { preventAgentSendTargetOutsideDismiss } from '../annotate/prevent-agent-send-target-outside-dismiss'
 import type { GrabIntent } from '../describe-page/browser-page-types'
@@ -97,22 +101,24 @@ export function BrowserPageChromeBanners({
                      'auto.components.browser.pane.BrowserPane.b733a91bd9',
                      'Add feedback for the selected element.'
                    )
-                  : browserAnnotationsLength === 1
-                    ? translate(
-                        'auto.components.browser.pane.BrowserPane.074f0ed10b',
-                        '{{value0}} annotation ready. Select another element or copy all feedback.',
-                        { value0: browserAnnotationsLength }
-                      )
-                    : browserAnnotationsLength > 0
+                  : isBrowserAnnotationPageFull(browserAnnotationsLength)
+                    ? browserAnnotationLimitMessage()
+                    : browserAnnotationsLength === 1
                      ? translate(
-                          'auto.components.browser.pane.BrowserPane.a2164a6e5a',
-                          '{{value0}} annotations ready. Select another element or copy all feedback.',
+                          'auto.components.browser.pane.BrowserPane.074f0ed10b',
+                          '{{value0}} annotation ready. Select another element or copy all feedback.',
                          { value0: browserAnnotationsLength }
                        )
-                      : translate(
-                          'auto.components.browser.pane.BrowserPane.777b5bc4ec',
-                          'Click an element to add feedback for the agent.'
-                        )
+                      : browserAnnotationsLength > 0
+                        ? translate(
+                            'auto.components.browser.pane.BrowserPane.a2164a6e5a',
+                            '{{value0}} annotations ready. Select another element or copy all feedback.',
+                            { value0: browserAnnotationsLength }
+                          )
+                        : translate(
+                            'auto.components.browser.pane.BrowserPane.777b5bc4ec',
+                            'Click an element to add feedback for the agent.'
+                          )
                : grab.state === 'confirming'
                  ? translate(
                      'auto.components.browser.pane.BrowserPane.e852e20cea',
@@ -16955,7 +16955,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "Kopiert",
                      "c937229f19": "Screenshot erstellt",
-                      "1f5cb19034": "Anmerkung hinzugefügt"
+                      "1f5cb19034": "Anmerkung hinzugefügt",
+                      "limitReached": "Limit von {{limit}} Anmerkungen erreicht. Senden, entfernen oder löschen Sie einige, um weitere hinzuzufügen."
                    }
                  }
                }
@@ -17382,7 +17382,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "Copied",
                      "c937229f19": "Screenshotted",
-                      "1f5cb19034": "Annotation added"
+                      "1f5cb19034": "Annotation added",
+                      "limitReached": "Limit of {{limit}} annotations reached. Send, delete or clear some to add more."
                    }
                  }
                }
@@ -16976,7 +16976,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "Copiado",
                      "c937229f19": "Captura tomada",
-                      "1f5cb19034": "Anotación añadida"
+                      "1f5cb19034": "Anotación añadida",
+                      "limitReached": "Se alcanzó el límite de {{limit}} anotaciones. Envíe, elimine o borre algunas para añadir más."
                    }
                  }
                }
@@ -17200,7 +17200,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "Copied",
                      "c937229f19": "Capture effectuée",
-                      "1f5cb19034": "Annotation ajoutée"
+                      "1f5cb19034": "Annotation ajoutée",
+                      "limitReached": "Limite de {{limit}} annotations atteinte. Envoyez, supprimez ou effacez-en pour en ajouter d’autres."
                    }
                  }
                }
@@ -16572,7 +16572,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "コピーしました",
                      "c937229f19": "スクリーンショット",
-                      "1f5cb19034": "注釈を追加しました"
+                      "1f5cb19034": "注釈を追加しました",
+                      "limitReached": "注釈は {{limit}} 件までです。追加するには、いくつか送信、削除、またはクリアしてください。"
                    }
                  }
                }
@@ -16572,7 +16572,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "복사됨",
                      "c937229f19": "스크린샷됨",
-                      "1f5cb19034": "주석이 추가됨"
+                      "1f5cb19034": "주석이 추가됨",
+                      "limitReached": "주석은 최대 {{limit}}개까지 추가할 수 있습니다. 더 추가하려면 일부를 보내거나 삭제하거나 지우세요."
                    }
                  }
                }
@@ -17018,7 +17018,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "Copiado",
                      "c937229f19": "Capturado",
-                      "1f5cb19034": "Anotação adicionada"
+                      "1f5cb19034": "Anotação adicionada",
+                      "limitReached": "Limite de {{limit}} anotações atingido. Envie, exclua ou limpe algumas para adicionar mais."
                    }
                  }
                }
@@ -17457,7 +17457,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "Скопировано",
                      "c937229f19": "Снимок сделан",
-                      "1f5cb19034": "Заметка добавлена"
+                      "1f5cb19034": "Заметка добавлена",
+                      "limitReached": "Достигнут лимит в {{limit}} аннотаций. Отправьте, удалите или очистите часть из них, чтобы добавить новые."
                    }
                  }
                }
@@ -16941,7 +16941,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "Kopyalandı",
                      "c937229f19": "Ekran görüntüsü alındı",
-                      "1f5cb19034": "Ek açıklama eklendi"
+                      "1f5cb19034": "Ek açıklama eklendi",
+                      "limitReached": "{{limit}} ek açıklama sınırına ulaşıldı. Daha fazla eklemek için bazılarını gönderin, silin veya temizleyin."
                    }
                  }
                }
@@ -16727,7 +16727,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "已複製",
                      "c937229f19": "已截圖",
-                      "1f5cb19034": "新增了註解"
+                      "1f5cb19034": "新增了註解",
+                      "limitReached": "已達到 {{limit}} 則註解的上限。請傳送、刪除或清除一些後再新增。"
                    }
                  }
                }
@@ -16572,7 +16572,8 @@
                    "annotations": {
                      "0c7b9b2b7a": "已复制",
                      "c937229f19": "截图",
-                      "1f5cb19034": "添加了注释"
+                      "1f5cb19034": "添加了注释",
+                      "limitReached": "已达到 {{limit}} 条注释的上限。请发送、删除或清除一些后再添加。"
                    }
                  }
                }
@@ -1,5 +1,4 @@
 import { afterEach, describe, expect, it, vi } from 'vitest'
-import { GRAB_BUDGET } from '../../../../shared/browser-grab-types'
 import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch'
 import { layoutApplyTestTransport } from '@/runtime/layout-apply-test-transport.test-fixture'
 import { createBrowserMockApi } from './browser-slice-test-harness'
@@ -60,39 +59,19 @@ describe('saved browser note lifetime and marker eligibility', () => {
    expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual(['fresh'])
  })

-  it('shares the 20-note budget across URLs and bounds marker IDs to retained notes', async () => {
+  it('prunes marker IDs when notes are deleted or cleared', async () => {
    const { store, pageId } = await createAnnotatedPage()
-    for (let index = 0; index < GRAB_BUDGET.annotationsMaxPerPage + 3; index++) {
-      const url = `https://example.com/page-${index % 2}`
-      store.getState().setBrowserPageUrl(pageId, url)
-      store.getState().invalidateBrowserPageAnnotationGeometry(pageId)
-      const note = makeAnnotation(pageId, `note-${index}`)
-      note.payload.page.sanitizedUrl = url
-      store.getState().addBrowserPageAnnotation(note)
-    }
-    const saved = store.getState().browserAnnotationsByPageId[pageId]
-    const eligible = store.getState().browserAnnotationMarkerIdsByPageId[pageId]
-    expect(saved).toHaveLength(20)
-    expect(saved[0].id).toBe('note-3')
-    expect(new Set(saved.map((note) => note.payload.page.sanitizedUrl)).size).toBe(2)
-    expect(eligible).toEqual(['note-22'])
-    expect(eligible.every((id) => saved.some((note) => note.id === id))).toBe(true)
+    store.getState().addBrowserPageAnnotation(makeAnnotation(pageId, 'second'))
+    expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual([
+      'annotation-1',
+      'second'
+    ])

-    store.getState().deleteBrowserPageAnnotation(pageId, 'note-22')
-    expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toBeUndefined()
+    store.getState().deleteBrowserPageAnnotation(pageId, 'second')
+    expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual(['annotation-1'])
    store.getState().clearBrowserPageAnnotations(pageId)
    expect(store.getState().browserAnnotationsByPageId[pageId]).toBeUndefined()
-  })
-
-  it('prunes eligible IDs when the oldest current-document notes are evicted', async () => {
-    const { store, pageId } = await createAnnotatedPage()
-    for (let index = 0; index < 22; index++) {
-      store.getState().addBrowserPageAnnotation(makeAnnotation(pageId, `note-${index}`))
-    }
-    expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toEqual(
-      store.getState().browserAnnotationsByPageId[pageId].map((note) => note.id)
-    )
-    expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toHaveLength(20)
+    expect(store.getState().browserAnnotationMarkerIdsByPageId[pageId]).toBeUndefined()
  })

  it('removes an accepted delivery snapshot after navigation while preserving later edits and additions', async () => {
@@ -611,7 +611,7 @@ describe('createBrowserSlice annotations', () => {
    expect(store.getState().setTabLabel).not.toHaveBeenCalled()
  })

-  it('caps stored browser annotations per page', async () => {
+  it('refuses browser annotations past the per-page cap instead of dropping the oldest', async () => {
    const { store, main } = await createTestStoreDrawnFromMain()
    const tab = store.getState().createBrowserTab('wt-1', 'https://example.com')
    await main.settle()
@@ -626,7 +626,8 @@ describe('createBrowserSlice annotations', () => {

    const annotations = store.getState().browserAnnotationsByPageId[pageId] ?? []
    expect(annotations).toHaveLength(GRAB_BUDGET.annotationsMaxPerPage)
-    expect(annotations[0]?.id).toBe('annotation-3')
+    expect(annotations[0]?.id).toBe('annotation-0')
+    expect(annotations.at(-1)?.id).toBe(`annotation-${GRAB_BUDGET.annotationsMaxPerPage - 1}`)
  })

  it('sanitizes persistent annotation payloads at the store boundary', async () => {
@@ -1,4 +1,4 @@
-import { GRAB_BUDGET } from '../../../../../shared/browser-grab-types'
+import { isBrowserAnnotationPageFull } from '../../../../../shared/browser-grab-types'
 import type {
  BrowserSlice,
  BrowserSliceGet,
@@ -79,9 +79,11 @@ export function createBrowserPageMetadataActions(
    addBrowserPageAnnotation: (annotation) =>
      set((s) => {
        const existing = s.browserAnnotationsByPageId[annotation.browserPageId] ?? []
-        const next = [...existing, sanitizeBrowserPageAnnotation(annotation)].slice(
-          -GRAB_BUDGET.annotationsMaxPerPage
-        )
+        // A full page refuses new notes; evicting the oldest would lose feedback unseen.
+        if (isBrowserAnnotationPageFull(existing.length)) {
+          return s
+        }
+        const next = [...existing, sanitizeBrowserPageAnnotation(annotation)]
        return {
          browserAnnotationsByPageId: {
            ...s.browserAnnotationsByPageId,
@@ -179,6 +179,10 @@ export const GRAB_BUDGET = {
  screenshotMaxBytes: 2 * 1024 * 1024
 } as const

+export function isBrowserAnnotationPageFull(annotationCount: number): boolean {
+  return annotationCount >= GRAB_BUDGET.annotationsMaxPerPage
+}
+
 /** Only these attribute names are included in the payload by default. */
 export const GRAB_SAFE_ATTRIBUTE_NAMES = new Set([
  'id',