Skip to content

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

Merged
backnotprop merged 1 commit into
mainfrom
fix/mobile-s1-followup
Oct 8, 2026
Merged

backnotprop merged 1 commit into
mainfrom
fix/mobile-s1-followup

Conversation

@backnotprop

Copy link
Copy Markdown
Owner

The story

The independent review of PR 1777 left six items, and plannotator-ops asked for one more. This PR fixes six of the seven with the smallest change each. For the seventh (forged selections from an agent's page), it shows the surface cannot drop them without dropping every HTML pin and writes the shell's check into the contract.

What changed, item by item

  1. Markdown links need a real tap. packages/inbox/surface/SurfaceApp.tsx: the main frame's link handler returns unless event.isTrusted. Proof: a scripted click() on the markdown link sends no link, after a real tap on it does.
  2. A forged plannotator-bridge-selection from an agent's page. Not dropped by the surface, deliberately. Markdown and diagram drafts already come only from the surface's own DOM events. The HTML viewer's bridge runs in the agent page's own realm, and a tap inside that frame never reaches the main frame (shown in WebKit on PR 1777). So the surface cannot tell a real pin or selection from a forged one, and cannot drop them without dropping every pin (4.3). Contract section 5 now says so. 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), the same way it takes links through .linkActivated.
  3. open_attachment.focus on markdown. SurfaceDocument.tsx: after the stored marks are painted (the frame after mount), the focused mark is scrolled to the middle of the screen. The painted flag is now set inside the frame callback, so a cancelled frame no longer skips the paint. Proof: a new test opens the plan at text scale 2.5 with focus set to the saved comment. It asserts the document scrolled and the mark is on screen, and fails with the scroll removed. M2's "3 annotations" sheet relies on this; it lands here first. HTML and diagram focus are unchanged.
  4. 6.2's diff box. packages/inbox/surface/surface.css, under .sf-gcard only: a file's diff box takes the height its diff has, up to 70% of the screen, instead of the desktop's line-count estimate. A short diff has no blank band (the 4-line package.json box went from 317 px to 191 px); a longer diff still scrolls inside its box, where the renderer draws what is in view (checked by hand with a 300-line file). Nothing changes in the guide chain, so the guides.show manifest stays in sync. Proof: the guide test asserts the box is within 60 px of the diff it holds.
  5. The agent page's network probe. tests/e2e/surface.spec.ts: it fetches the live Inbox's /api/inbox/threads with mode: 'no-cors', so only the inherited CSP can refuse it (the review: 127.0.0.1:1 refuses anyway).
  6. Label comments. packages/core/inbox-surface-bridge.ts (SurfacePin) and packages/ui/types.ts (HostDraft.label) no longer say "div.btnx"; they name Plannotator's pinpoint label ("Button", a heading's words), as the contract does.
  7. CI paths. .github/workflows/inbox-e2e.yml runs on apps/hook/package.json too (it carries the surface's build step), for pull requests and pushes.

Proofs

bunx playwright test -c tests/e2e/playwright.config.ts surface.spec.ts (WebKit) after bun run build:review && bun run build:hook and the compiled binary: 9 passed. New: the focus test; asserts added for the scripted link click, the diff box and the live-Inbox fetch. PNGs refreshed in research/operational/mobile-proofs/S1-2026-10-08/.

Gates run

  • bun run typecheck: green.
  • bun test packages apps/hook tests scripts under a temp data dir: 5858 pass, 1 fail. The failure was packages/server/inbox.test.ts "restart to update" (untouched here); it passed twice when run alone.
  • tests/entry-assets.test.ts, tests/favicon-surfaces.test.ts: pass.
  • bun run --cwd packages/ui smoke:package: pass (ui changed only a comment, so the DOM suite was not rerun).
  • bun run check:release-version: consistent.
  • Gitleaks over the range: no leaks.

Leaves out

The shell's touch check and link navigation (M2, contract section 5). focus on HTML and diagram attachments.

Deviations

Item 2 is answered in the contract instead of in surface code, for the reason above. No limits added.

A markdown link goes to the shell only for a trusted tap; a file opened at
one mark scrolls to it once the marks are painted; 6.2's diff box fits its
diff; the agent page's fetch targets the live Inbox (no-cors, so only the
CSP refuses it); the pin label comments say "Button"; inbox-e2e runs on
apps/hook/package.json. Contract section 5: an HTML page's pins and
selections are page-reported, checked by the shell in M2.
@backnotprop
backnotprop merged commit 1d100a7 into main Oct 8, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant