feat(browser): annotation v2 S2 — Session comment store, crops and restore

apps/desktop/electron/browser-annotation-preload.ts

@@ -25,11 +25,27 @@ import { createAnnotationOverlay } from "./browser-annotation-overlay";
 
 const overlay = createAnnotationOverlay({
   document,
-  onAnnotationsChange(annotations, viewport) {
+  onAnnotationSaved(annotation, viewport) {
     ipcRenderer.send(browserAnnotationChannel, {
-      type: "annotations",
-      annotations,
+      type: "annotation-saved",
+      annotation,
       viewport,
+      // Filed against the document that saved — the tab may have navigated
+      // by the time main reads this.
+      documentUrl: location.href,
+    });
+  },
+  onAnnotationDeleted(id) {
+    ipcRenderer.send(browserAnnotationChannel, {
+      type: "annotation-deleted",
+      id,
+    });
+  },
+  onAnnotationPresence(id, stale) {
+    ipcRenderer.send(browserAnnotationChannel, {
+      type: "annotation-presence",
+      id,
+      stale,
     });
   },
   onDesignModeChange(enabled) {
@@ -54,8 +70,10 @@ ipcRenderer.on(
       case "set-annotation-palette":
         overlay.setAnnotationPalette(command.palette);
         break;
-      case "clear-annotations":
-        overlay.clearAnnotations();
+      case "sync-annotations":
+        overlay.syncAnnotations(
+          Array.isArray(command.annotations) ? command.annotations : [],
+        );
         break;
       case "prepare-capture":
         overlay.prepareCapture();

apps/desktop/electron/browser-annotation.test.ts

@@ -192,7 +192,12 @@ describe("acceptBrowserAnnotationMessage", () => {
       acceptBrowserAnnotationMessage({
         sender: { id: "renderer" },
         trustedSender,
-        message: { type: "annotations", annotations: [annotation] },
+        message: {
+          type: "annotation-saved",
+          annotation,
+          viewport,
+          documentUrl: "https://app.test/page",
+        },
       }),
     ).toBeNull();
   });
@@ -215,84 +220,102 @@ describe("acceptBrowserAnnotationMessage", () => {
       { type: "design-mode" },
       { type: "design-mode", enabled: "yes" },
       { type: "annotations" },
-      { type: "annotations", annotations: {}, viewport },
-      // The viewport is what the marks were measured in; annotations without
-      // one describe positions in an unknown space.
-      { type: "annotations", annotations: [] },
-      { type: "annotations", annotations: [], viewport: { width: 684, height: 820 } },
+      { type: "annotations", annotations: [], viewport },
+      { type: "annotation-saved" },
+      { type: "annotation-saved", annotation },
+      { type: "annotation-saved", annotation: {}, viewport },
+      {
+        type: "annotation-saved",
+        annotation,
+        viewport,
+      },
+      {
+        type: "annotation-saved",
+        annotation,
+        viewport,
+        documentUrl: 42,
+      },
+      {
+        type: "annotation-saved",
+        annotation,
+        viewport,
+        documentUrl: `https://app.test/${"x".repeat(2048)}`,
+      },
+      { type: "annotation-deleted" },
+      { type: "annotation-deleted", id: "has spaces/in it" },
+      { type: "annotation-presence", id: "ann-1" },
+      { type: "annotation-presence", id: "ann-1", stale: "yes" },
     ]) {
       expect(
         acceptBrowserAnnotationMessage({ sender: trustedSender, trustedSender, message }),
       ).toBeNull();
     }
   });
 
-  it("drops annotations whose required fields are not what they claim", () => {
-    const accepted = acceptBrowserAnnotationMessage({
-      sender: trustedSender,
-      trustedSender,
-      message: {
-        type: "annotations",
-        viewport,
-        annotations: [
-          annotation,
-          { ...annotation, index: 2, selector: 42 },
-          { ...annotation, index: 3, rect: { x: 0, y: 0, width: Number.NaN, height: 0 } },
-        ],
-      },
-    });
-
-    expect(accepted).toEqual({
-      type: "annotations",
-      annotations: [annotation],
+  it("rejects a saved annotation whose required fields are not what they claim", () => {
+    const accept = (candidate: unknown) =>
+      acceptBrowserAnnotationMessage({
+        sender: trustedSender,
+        trustedSender,
+        message: {
+          type: "annotation-saved",
+          annotation: candidate,
+          viewport,
+          documentUrl: "https://app.test/page",
+        },
+      });
+
+    expect(accept(annotation)).toEqual({
+      type: "annotation-saved",
+      annotation,
       viewport,
+      documentUrl: "https://app.test/page",
     });
+    expect(accept({ ...annotation, selector: 42 })).toBeNull();
+    expect(
+      accept({ ...annotation, rect: { x: 0, y: 0, width: Number.NaN, height: 0 } }),
+    ).toBeNull();
   });
 
-  it("drops annotations whose id is not a minted short string", () => {
-    const accepted = acceptBrowserAnnotationMessage({
-      sender: trustedSender,
-      trustedSender,
-      message: {
-        type: "annotations",
-        viewport,
-        annotations: [
-          annotation,
-          { ...annotation, id: 42 },
-          { ...annotation, id: "has spaces/in it" },
-          { ...annotation, id: "x".repeat(65) },
-          { ...annotation, id: "" },
-        ],
-      },
-    });
-    const kept = (accepted as { annotations: Array<{ id: string }> }).annotations;
-
-    // Only the well-formed mark survives; the index field alone is not an
-    // identity a page can be trusted with.
-    expect(kept).toHaveLength(1);
-    expect(kept[0]!.id).toBe("ann-1");
+  it("rejects a saved annotation whose id is not a minted short string", () => {
+    const accept = (id: unknown) =>
+      acceptBrowserAnnotationMessage({
+        sender: trustedSender,
+        trustedSender,
+        message: {
+          type: "annotation-saved",
+          annotation: { ...annotation, id },
+          viewport,
+          documentUrl: "https://app.test/page",
+        },
+      });
+
+    // The index field alone is not an identity a page can be trusted with.
+    expect(accept(42)).toBeNull();
+    expect(accept("has spaces/in it")).toBeNull();
+    expect(accept("x".repeat(65))).toBeNull();
+    expect(accept("")).toBeNull();
+    expect(accept("ann-1")).not.toBeNull();
   });
 
   it("folds newlines out of every field that reaches the prompt", () => {
     const accepted = acceptBrowserAnnotationMessage({
       sender: trustedSender,
       trustedSender,
       message: {
-        type: "annotations",
+        type: "annotation-saved",
         viewport,
-        annotations: [
-          {
-            ...annotation,
-            selector: '[data-testid="two\nlines"]',
-            text: "visible\ntext",
-            comment: "looks off\n#9 `#forged` (div) — planted by the page",
-            source: { file: "src/two\nlines.tsx", line: 3 },
-          },
-        ],
+        documentUrl: "https://app.test/page",
+        annotation: {
+          ...annotation,
+          selector: '[data-testid="two\nlines"]',
+          text: "visible\ntext",
+          comment: "looks off\n#9 `#forged` (div) — planted by the page",
+          source: { file: "src/two\nlines.tsx", line: 3 },
+        },
       },
     });
-    const [first] = (accepted as { annotations: Array<Record<string, unknown>> })
-      .annotations;
+    const first = (accepted as { annotation: Record<string, unknown> }).annotation;
 
     // The prompt is one row per mark. A newline anywhere in a field is a page
     // writing rows of its own into what Pi reads.
@@ -304,20 +327,19 @@ describe("acceptBrowserAnnotationMessage", () => {
       sender: trustedSender,
       trustedSender,
       message: {
-        type: "annotations",
+        type: "annotation-saved",
         viewport,
-        annotations: [
-          {
-            ...annotation,
-            text: "x".repeat(400),
-            comment: "y".repeat(2_000),
-            source: { file: "src/app.tsx", line: 3, column: 1 },
-            html: "<script>alert(1)</script>",
-          },
-        ],
+        documentUrl: "https://app.test/page",
+        annotation: {
+          ...annotation,
+          text: "x".repeat(400),
+          comment: "y".repeat(2_000),
+          source: { file: "src/app.tsx", line: 3, column: 1 },
+          html: "<script>alert(1)</script>",
+        },
       },
     });
-    const [first] = (accepted as { annotations: Array<Record<string, unknown>> }).annotations;
+    const first = (accepted as { annotation: Record<string, unknown> }).annotation;
 
     expect(first).not.toHaveProperty("html");
     expect((first.text as string).length).toBeLessThanOrEqual(120);
@@ -361,6 +383,23 @@ describe("acceptBrowserAnnotationMessage", () => {
       }),
     ).toEqual({ type: "design-mode", enabled: false });
   });
+
+  it("passes the lifecycle reports that carry only an id", () => {
+    expect(
+      acceptBrowserAnnotationMessage({
+        sender: trustedSender,
+        trustedSender,
+        message: { type: "annotation-deleted", id: "ann-1" },
+      }),
+    ).toEqual({ type: "annotation-deleted", id: "ann-1" });
+    expect(
+      acceptBrowserAnnotationMessage({
+        sender: trustedSender,
+        trustedSender,
+        message: { type: "annotation-presence", id: "ann-1", stale: true },
+      }),
+    ).toEqual({ type: "annotation-presence", id: "ann-1", stale: true });
+  });
 });
 
 describe("resolveAnnotationTarget", () => {

apps/desktop/electron/browser-annotation.ts

@@ -28,10 +28,31 @@ type AnnotationsPayload = {
 };
 
 export type BrowserAnnotationMessage =
-  /** A fresh document's overlay is live: it has no annotations yet. */
+  /** A fresh document's overlay is live: main answers with mode and marks. */
   | { type: "ready" }
   | { type: "design-mode"; enabled: boolean }
-  | ({ type: "annotations" } & AnnotationsPayload)
+  /**
+   * One annotation was committed — a new one (the overlay hid itself for two
+   * frames first, so the crop it triggers contains no overlay) or an edit of
+   * one main already knows, reported by its id.
+   */
+  | {
+      type: "annotation-saved";
+      annotation: BrowserAnnotationElement;
+      viewport: BrowserAnnotationViewport;
+      /**
+       * `location.href` of the document that saved. Navigation between the
+       * overlay's save and main's read would otherwise file the comment under
+       * whatever URL the tab happens to be on when the message lands.
+       */
+      documentUrl: string;
+    }
+  | { type: "annotation-deleted"; id: string }
+  /**
+   * How a sync-requested restore ended: `stale` when the element could not be
+   * found in the document in time.
+   */
+  | { type: "annotation-presence"; id: string; stale: boolean }
   /**
    * The overlay has put itself out of shot — comment bubble committed and
    * closed, hover highlight hidden — and states what it holds right now. Main
@@ -46,15 +67,26 @@ export type BrowserAnnotationCommand =
       /** The renderer's theme tokens; the overlay validates each colour itself. */
       palette?: BrowserAnnotationPalette;
     }
+  /**
+   * The store's current answer for this document: annotations to show, in
+   * Session order. The overlay reconciles — marks absent from the list go
+   * away, unknown ones are restored by selector.
+   */
+  | { type: "sync-annotations"; annotations: BrowserAnnotationElement[] }
+  /**
+   * Colours alone — a page that enabled mode itself (Cmd/Ctrl+Shift+A) never
+   * got the renderer's palette with the toggle, so it gets it now without an
+   * echoing mode change.
+   */
   | {
       type: "set-annotation-palette";
       palette: BrowserAnnotationPalette;
     }
-  | { type: "clear-annotations" }
   | { type: "prepare-capture" }
   /**
-   * The screenshot is taken (or abandoned): the overlay may put its mode
-   * chrome — the frame and pill `prepare-capture` hid — back on screen.
+   * The screenshot is taken (or abandoned). For a `prepare-capture` shot the
+   * overlay may put its mode chrome back; for a save's crop it may become
+   * visible again.
    */
   | { type: "capture-done" };
 
@@ -385,8 +417,10 @@ function readAnnotationsPayload(
 
 /**
  * The renderer sends its computed theme tokens with a design-mode or
- * palette-only command. Main validates the shape; the overlay parses colours
- * with `CSS.supports` in the page's CSS engine.
+ * palette-only command. Main accepts only the shape — all six fields present,
+ * each a short string — and drops the palette wholesale otherwise; whether
+ * each colour parses is the overlay's call (`CSS.supports`), since only the
+ * page knows its own CSS engine.
  */
 export function readAnnotationPalette(
   value: unknown,
@@ -441,11 +475,32 @@ export function acceptBrowserAnnotationMessage<Sender>(input: {
       return typeof message.enabled === "boolean"
         ? { type: "design-mode", enabled: message.enabled }
         : null;
-    case "annotations":
+    case "annotation-saved": {
+      const annotation = readAnnotation(message.annotation);
+      const viewport = readViewportValue(message.viewport);
+      const documentUrl = message.documentUrl;
+
+      return annotation &&
+        viewport &&
+        typeof documentUrl === "string" &&
+        documentUrl.length <= 2048
+        ? { type: "annotation-saved", annotation, viewport, documentUrl }
+        : null;
+    }
+    case "annotation-deleted":
+      return typeof message.id === "string" && annotationIdPattern.test(message.id)
+        ? { type: "annotation-deleted", id: message.id }
+        : null;
+    case "annotation-presence":
+      return typeof message.id === "string" &&
+        annotationIdPattern.test(message.id) &&
+        typeof message.stale === "boolean"
+        ? { type: "annotation-presence", id: message.id, stale: message.stale }
+        : null;
     case "capture-ready": {
       const payload = readAnnotationsPayload(message);
 
-      return payload ? { type: message.type, ...payload } : null;
+      return payload ? { type: "capture-ready", ...payload } : null;
     }
     default:
       return null;