Repository navigation
Inbox mobile S1 follow-up: the review's items - #1782
Merged
Merged
Conversation
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.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
packages/inbox/surface/SurfaceApp.tsx: the main frame's link handler returns unlessevent.isTrusted. Proof: a scriptedclick()on the markdown link sends nolink, after a real tap on it does.plannotator-bridge-selectionfrom 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 apin,selectionordraftfrom 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.open_attachment.focuson 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 withfocusset 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 diagramfocusare unchanged.packages/inbox/surface/surface.css, under.sf-gcardonly: 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-linepackage.jsonbox 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.tests/e2e/surface.spec.ts: it fetches the live Inbox's/api/inbox/threadswithmode: 'no-cors', so only the inherited CSP can refuse it (the review:127.0.0.1:1refuses anyway).packages/core/inbox-surface-bridge.ts(SurfacePin) andpackages/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..github/workflows/inbox-e2e.ymlruns onapps/hook/package.jsontoo (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) afterbun run build:review && bun run build:hookand 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 inresearch/operational/mobile-proofs/S1-2026-10-08/.Gates run
bun run typecheck: green.bun test packages apps/hook tests scriptsunder a temp data dir: 5858 pass, 1 fail. The failure waspackages/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.Leaves out
The shell's touch check and link navigation (M2, contract section 5).
focuson HTML and diagram attachments.Deviations
Item 2 is answered in the contract instead of in surface code, for the reason above. No limits added.