Inbox mobile S1 follow-up: the review's items

.github/workflows/inbox-e2e.yml

@@ -39,6 +39,7 @@ on:
       - 'packages/server/guide/**'
       - 'packages/core/guide*'
       - 'apps/hook/server/index.ts'
+      - 'apps/hook/package.json'
       - 'apps/hook/server/inbox*'
       - 'build/**'
       - 'package.json'
@@ -70,6 +71,7 @@ on:
       - 'packages/server/guide/**'
       - 'packages/core/guide*'
       - 'apps/hook/server/index.ts'
+      - 'apps/hook/package.json'
       - 'apps/hook/server/inbox*'
       - 'build/**'
       - 'package.json'

adr/implementation/inbox-mobile.md

@@ -337,6 +337,8 @@ Annotations are Plannotator's `Annotation` (`packages/ui/types.ts`) inside the I
 
 **Links in an agent's HTML page are not a bridge message (S1 review, 2026-10-08).** The `link` message's only source is the main frame's own rendered markdown, whose taps the surface sees itself. A link in an agent's page is a navigation the shell decides: the page's script can forge any message the viewer's frame sends, and a tap inside that frame never reaches the main frame (proved in WebKit), so the surface cannot tell a real tap from a forged one. In M2 the shell takes it in `WKNavigationDelegate.decidePolicyFor` with `navigationType == .linkActivated` on the agent's frame, a real-tap signal the page cannot forge, cancels the in-frame load and opens the URL in `SFSafariViewController`. Note for M2: today Plannotator's HTML viewer bridge cancels every non-fragment link click inside the frame (`preventDefault`) and reports it as a frame message, so no navigation reaches the delegate until a ui seam lets the frame navigate.
 
+**Pins and selections from an agent's HTML page are page-reported (S1 follow-up, 2026-10-08).** For the same reason, the surface cannot tell a pin or a text selection the person made in an agent's page from one the page's script forged (the viewer's bridge runs in the page's own realm): a forged one reaches the shell as an ordinary `pin` or `selection`, with a label the page chose. The surface cannot drop them without dropping every pin (4.3). In M2 the shell takes a `pin`, `selection` or `draft` from an HTML attachment only within about a second of its own touch on the web view (a gesture recognizer that observes touches without cancelling them); markdown and diagrams are drawn in the main frame from the surface's own events, so their drafts need no such check. Links in the surface's own markdown go out only for a real tap (`isTrusted`).
+
 `open_section` and `section` were added by S1 (2026-10-08): 6.1 and 6.2 put the guide's title and its close or back button in the shell's bar, and Continue, Previous, Reviewed and Next in the document, so each side has to tell the other where the guide is. A `selection` with both fields null follows only a selection draft; a pin or a part whose draft closes sends nothing. A `pin`'s `label` is the element's name as Plannotator's pinpoint names it ("Button", a heading's words), and `selector` is the draft's `htmlAnchor.selector`. A `draft`'s `label` is the quote's first words for a block and "Pick a host (node E)" for a diagram part; `kind` is `block` for a whole-diagram comment too. The surface is `apps/hook/dist/surface.html`; its CSP is `default-src 'none'` with `connect-src 'none'`, and lets an HTML page's own folder load from `plannotator-asset:` (script, style, image, font, media, frame and `base-uri`), since the page's frame inherits the policy.
 
 A draft is an `Annotation` with an empty `text`. The shell fills the person's words into `text`, saves `{ attachment_id, version, annotation }` through the door (7.17), and answers with `commit_annotation`. Edit (4.3) is the same save with the same annotation id and a new key. HTML-asset tokens live in the Inbox's memory, so after the Inbox restarts an asset answers 404; the shell then fetches `view` again.

packages/core/inbox-surface-bridge.ts

@@ -143,7 +143,7 @@ export interface SurfaceSelection {
 export interface SurfacePin {
   v: 1;
   type: "pin";
-  /** `label` is the element as the sheet names it ("div.btnx"); `selector` finds it again. */
+  /** `label` is the element as the sheet names it (Plannotator's pinpoint label: "Button", a heading's words); `selector` finds it again. */
   target: { label: string; selector: string };
   draft: SurfaceAnnotation;
 }

packages/inbox/surface/SurfaceApp.tsx

@@ -115,6 +115,8 @@ function Surface() {
   // has nowhere to go without a file of its own, so it does nothing.
   useEffect(() => {
     const onClick = (event: MouseEvent) => {
+      // A person's tap only: a scripted click on the document is not one.
+      if (!event.isTrusted) return;
       const link = (event.target as Element | null)?.closest?.('a[href]');
       if (!link || event.defaultPrevented) return;
       const href = link.getAttribute('href') ?? '';

packages/inbox/surface/SurfaceDocument.tsx

@@ -69,13 +69,21 @@ export const SurfaceDocument = forwardRef<SurfaceDocumentHandle, Props>(function
   const blocks = useMemo(() => attachmentBlocks(attachment.kind, open.text, attachment.name), [attachment.kind, attachment.name, open.text]);
 
   // Paint the stored highlights once the document is drawn; later ones arrive through commit.
+  // The shell's "3 annotations" sheet opens a file at one mark (`focus`):
+  // once the marks are painted, that one is brought to the middle of the screen.
   const painted = useRef(false);
   useEffect(() => {
     if (!isText || painted.current) return;
-    painted.current = true;
-    const id = requestAnimationFrame(() => viewerRef.current?.applySharedAnnotations(annotations.filter(annotationOwnsHighlight)));
+    const id = requestAnimationFrame(() => {
+      painted.current = true;
+      viewerRef.current?.applySharedAnnotations(annotations.filter(annotationOwnsHighlight));
+      if (open.focus) {
+        const mark = document.querySelector(`[data-highlight-id="${CSS.escape(open.focus)}"], [data-bind-id="${CSS.escape(open.focus)}"]`);
+        mark?.scrollIntoView({ block: 'center' });
+      }
+    });
     return () => cancelAnimationFrame(id);
-  }, [isText, annotations]);
+  }, [isText, annotations, open.focus]);
 
   const dropDraft = () => {
     draft.current?.cancel();

packages/inbox/surface/surface.css

@@ -319,3 +319,20 @@
     transition: none;
   }
 }
+
+/* One section per screen: a file's diff box takes the height its diff has,
+   up to most of the screen, instead of the desktop's line-count estimate,
+   which leaves a blank band under a short diff at a phone's width. A longer
+   diff still scrolls inside its box, where the renderer draws what is in
+   view. */
+.sf-gcard [data-guide-code-view-mounted] {
+  height: auto !important;
+}
+.sf-gcard [data-guide-code-view-mounted] > div {
+  height: auto;
+}
+.sf-gcard [data-guide-code-view-mounted] > div > div {
+  height: auto;
+  max-height: 70vh;
+  contain: none;
+}

packages/ui/types.ts

@@ -537,7 +537,7 @@ export interface HostDraft {
    * tap, Comment mode, an HTML pin).
    */
   intent: 'selection' | 'compose';
-  /** What it is on, as a composer names it ("div.btnx"). Absent for text. */
+  /** What it is on, as a composer names it (Plannotator's pinpoint label: "Button", a heading's words). Absent for text. */
   label?: string;
   /**
    * Drop the draft: its pending highlight or pin goes. The host calls it when

tests/e2e/surface.spec.ts

@@ -131,7 +131,8 @@ const TICKET_PAGE = `<!doctype html>
   parent.postMessage({ type: 'plannotator-bridge-link-click', href: 'https://evil.example/forged' }, '*');
   parent.postMessage({ type: 'plannotator-bridge-link-click', href: 'mailto:evil@evil.example' }, '*');
   document.getElementById('forged').textContent = 'forged';
-  fetch('http://127.0.0.1:1/').then(function () { out.push('fetch: reached'); }, function () { out.push('fetch: refused'); }).then(function () {
+  // The live Inbox, no-cors: only the inherited CSP can refuse it.
+  fetch('__INBOX_THREADS__', { mode: 'no-cors' }).then(function () { out.push('fetch: reached'); }, function () { out.push('fetch: refused'); }).then(function () {
     document.getElementById('probe').textContent = out.join(' | ');
   });
 </script>
@@ -167,6 +168,8 @@ interface World {
   inbox: ShellMessage[];
   attachments: { plan: any; ticket: any; flow: any };
   guide: { thread_id: string; message_id: string };
+  /** The comment the markdown test saved, for the focus test. */
+  savedMarkId: string;
 }
 
 let world: World;
@@ -195,7 +198,7 @@ async function inboxJson(path: string, init?: { method: string; body: unknown })
 }
 
 /** What the shell does to open a file: read its view through the door (here, the window's route) and hand it over. */
-async function openAttachment(attachment: any): Promise<void> {
+async function openAttachment(attachment: any, focus: string | null = null): Promise<void> {
   const view = await inboxJson(`/api/inbox/attachments/${attachment.id}/view`);
   const html = view.html === null ? null : String(view.html).replace(/<base href="\/api\/html-assets\/([^"/]+)\//, '<base href="plannotator-asset://inbox/api/html-assets/$1/');
   const thread = await inboxJson(`/api/inbox/threads/${view.attachment.message_id}/attachments`);
@@ -206,7 +209,7 @@ async function openAttachment(attachment: any): Promise<void> {
     text: view.text,
     html,
     annotations: thread.annotations.filter((r: any) => r.attachment_id === attachment.id),
-    focus: null,
+    focus,
   });
 }
 
@@ -311,7 +314,7 @@ test.beforeAll(async ({ browser }: { browser: Browser }) => {
   const docs = scratchProject(join(root, 'src'), 'docs-site');
   const ledger = scratchProject(join(root, 'src'), 'ledger');
   writeFileSync(join(billing, 'retry-plan.md'), PLAN);
-  writeFileSync(join(checkout, 'ticket-page.html'), TICKET_PAGE);
+  writeFileSync(join(checkout, 'ticket-page.html'), TICKET_PAGE.replace('__INBOX_THREADS__', new URL('/api/inbox/threads', url).href));
   writeFileSync(join(docs, 'install-flow.mmd'), FLOW);
   const claude = await SimAgent.connect({ binary, env, name: 'Claude Code', host: 'claude-code' });
   const codex = await SimAgent.connect({ binary, env, name: 'Codex', host: 'codex' });
@@ -358,6 +361,7 @@ test.beforeAll(async ({ browser }: { browser: Browser }) => {
     inbox,
     attachments: { plan: plan.attachments[0], ticket: ticket.attachments[0], flow: flow.attachments[0] },
     guide: { thread_id: guide.thread_id, message_id: guide.message_id },
+    savedMarkId: '',
   };
 });
 
@@ -466,6 +470,12 @@ test('markdown: a selection becomes a draft with its quote; saved, it is drawn;
   await page.locator('[data-surface-attachment] a', { hasText: 'the Stripe docs' }).click();
   expect(await next('link', from)).toMatchObject({ href: 'https://docs.stripe.com/error-low-level#idempotency' });
   expect(page.url()).toContain('surface.html');
+  // A scripted click on the same link is not a tap: nothing goes to the shell.
+  from = mark();
+  await page.evaluate(() => (document.querySelector('[data-surface-attachment] a[href^="https://docs.stripe.com"]') as HTMLElement).click());
+  await page.waitForTimeout(300);
+  expect(world.inbox.slice(from).filter((m) => m.main && m.message.type === 'link')).toEqual([]);
+  world.savedMarkId = again.draft.id;
 });
 
 test('HTML: a pin steps to its parent and back to its child, is saved and drawn; Interact and Annotate switch', async () => {
@@ -609,6 +619,13 @@ test('a guided review: sections, a tick, Continue, one section with Reviewed; th
   expect(await next('section', from)).toMatchObject({ message_id: world.guide.message_id, section: 1, sections: 2 });
   await expect(page.locator('[data-section-page="1"]')).toBeVisible();
   await expect(page.locator('[data-section-page="1"] .sf-gnavbtn', { hasText: '01' })).toBeVisible();
+  // The diff box fits its diff: no blank band under a short diff at a phone's width.
+  await expect(page.locator('[data-section-page="1"] diffs-container').first()).toBeVisible();
+  const slack = await page.evaluate(() => {
+    const box = document.querySelector('[data-section-page="1"] [data-guide-code-view-mounted]')!;
+    return box.getBoundingClientRect().height - box.querySelector('diffs-container')!.getBoundingClientRect().height;
+  });
+  expect(slack).toBeLessThan(60);
   await shot('09-guide-section');
 
   from = mark();
@@ -625,6 +642,20 @@ test('a guided review: sections, a tick, Continue, one section with Reviewed; th
   expect(message.guide_reviewed).toEqual([true, true]);
 });
 
+test("a file opened at one of its marks (the shell's annotations sheet) scrolls that mark into view", async () => {
+  const { page } = world;
+  // Large text, so the mark starts well below the first screen.
+  await appearance('light', 2.5);
+  await openAttachment(world.attachments.plan, world.savedMarkId);
+  const markEl = page.locator(`[data-surface-attachment] [data-highlight-id="${world.savedMarkId}"]`).first();
+  await expect(markEl).toBeVisible();
+  await expect.poll(() => page.evaluate(() => document.querySelector('.sf-docscroll')!.scrollTop)).toBeGreaterThan(0);
+  const box = (await markEl.boundingBox())!;
+  expect(box.y).toBeGreaterThan(0);
+  expect(box.y + box.height).toBeLessThan(852);
+  await appearance('light', 1);
+});
+
 test('theme and text size apply as the shell sends them', async () => {
   const { page } = world;
   await openAttachment(world.attachments.plan);