starred/orca - orca - Gitea: Git with a cup of tea

starred/orca

Fork 0

mirror of https://github.com/stablyai/orca.git synced 2026-09-22 16:02:32 +00:00

Commit Graph

Author SHA1 Message Date

Author	SHA1	Message	Date
Jinwoo Hong	a3c6d4266a	fix(mobile): admit https: images on the web shell's CSP (OTA phase C, ruling 27) (#21964 ) * fix(mobile): admit https: images on the web shell's CSP (OTA phase C, ruling 27) Native markdown and the native rich editor load images the author referenced by URL, so the page has to as well or a remote image is a blank where native paints a picture. `img-src` widens to `img-src 'self' data: https:` on both platforms; `script-src`, `connect-src`, `object-src`, `frame-src` and `child-src` do not move. `http:` stays out, and the pins say so directly rather than by absence: the Kotlin test's blanket `!contains("http")` could not survive `https:`, so both native pins now check `http:` (not a substring of `https:`) and check that `https:` appears in `img-src` and nowhere else, the same shape the `data:` pin already had. No behaviour change on released phones: the shell ships in no released tag (mobile-v0.0.9 predates it), so this reaches devices with the Phase E native build and not before. Neither native module has a CI job, so both ran locally: swiftc over the module plus MobileWebShellChecks, and `:orca-mobile-web-shell:testDebugUnitTest`. Both were confirmed red against the old directive first. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct what the sealed preview frame is stricter about The doc comment said the page was deliberately stricter than the native preview because it loads no remote image and runs no script. Since `img-src` gained `https:` only the script half is true: the frame loads a remote image exactly as the native WebView does. Says instead what an artifact's image URL now is -- a channel that fires on view and carries whatever its author encoded, with nothing dynamic behind it because no script runs -- and names `referrerPolicy` as what keeps the document's own origin out of the request. Comment only; no behaviour and no test moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): measure both halves of the preview frame's image fence "fetches nothing of the artifact that leaves the origin" stopped being what the sealed arm proves once `img-src` gained `https:`. The fixture's foreign origin is `http://127.0.0.1`, so its two images are refused on the scheme alone and only the font is refused by `font-src 'none'`. Renamed to say exactly that. The half that was missing is an https arm. Playwright route interception answers an `https://…invalid` origin in the page, so the arm needs no TLS server and no new dependency, and a request only reaches the handler if the policy let it out. Under the shipped header, on Chromium and WebKit, the `<img>` and the CSS background are both requested -- `img-src` governs a background too -- and the font still is not. `artifact()` takes the subresource origin; the links stay on the cleartext one so no existing navigation case changes. Red-first: with `img-src 'self' data:` put back into the parsed Kotlin policy, the new arm fails on both engines with `expected [] to deeply equal [ '/css-bg.png', '/img.png' ]`. The directive was restored byte-identical before this commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(scripts): split the preview frame's settling out of the render check The https arm pushed mobile-web-app-html-preview-render.test.mjs to 620 counted lines, over the 600 cap config/scripts carries. Split at a module boundary rather than bumped: the four wait-and-settle functions are rig mechanics with no assertion in them, and they now sit beside the diagnosis module they already reported through. `waitForLoadedFrame` and `settleAfterMount` are the two the render check calls; `waitForRecordedNavigation` and `settleWithoutNavigation` stay internal to the new module. Move only. Same 20 tests pass on both engines. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): send Referrer-Policy: no-referrer on the shell document `img-src https:` gave the page somewhere to send a request, and the document origin is `orca-mobile-web://<sessionId>/`, so a request that carries a referrer carries the session id to whatever host an artifact or a markdown document named. `referrerPolicy="no-referrer"` on the preview iframe does not cover it. Measured in the render rig against a permissive control policy: WebKit puts the embedder's URL on a srcdoc frame's image request despite the attribute, and Chromium sends none. So the guarantee belongs on the document, where one header covers every request the page makes, and it rides the document alone with the policy -- the referrer of a request is decided by the document that made it, so on a subresource response it would govern nothing. WKWebView under the custom scheme is unverified: the rig is Playwright WebKit over http, not WKWebView over `orca-mobile-web://`. The header is the hedge, and it costs nothing if that host never leaked. Pinned three ways, each confirmed red first: - Swift, exit 133 with the header removed. - Kotlin, MobileWebShellResponseHeadersTest "sends the policy on the document" FAILED at :17 with it removed. - The rig, through a new `readShellDocumentHeaders` that parses the Kotlin source the way `readShellCsp` does and throws rather than returning an empty map. With the value flipped to `unsafe-url` the WebKit arm fails `expected [ …(2) ] to deeply equal [ null, null ]`; with the line deleted the parse throws "could not parse the shell document headers". The rig's arm carries its own presence precondition: a third server serves the shipped policy with `unsafe-url`, so the WebKit reading is the header doing the work, and Chromium's null either way is pinned as the browser's behaviour rather than sold as evidence the header arrived. MobileHtmlPreview.web.tsx said the iframe attribute kept the origin out of the request. Corrected to name the header, since the measurement above is what disproved it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): quote the current directive where the old text was written down Three comments still read `img-src 'self' data:`, so a grep for the old directive found live prose that no longer matches the header. Each stays about `data:`, which is what those paths rest on; only the quoted policy changes. The two remaining hits in the repo are src/main/browser/doc-preview-protocol, which is the desktop preview's own policy and not this one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): name the surfaces img-src https: actually unblocks today The comment justified `https:` with markdown and the rich editor, and neither renders a remote image on the page. Verified in the tree: MobileMarkdown paints `![](...)` as a tappable link at both of its image branches and never mounts an Image, and it has no `.web` sibling, so that is what native does too; MobileRichMarkdownEditor.web.tsx is a 92-line multiline TextInput, still C7.6's plain source field. What the directive unblocks today is four surfaces, none of them overridden on the page: - MobileAgentIcon's favicon, a hardcoded `google.com/s2/favicons` URL, used by thirteen callers including the session header and the worktree rows; - MobileRepoIcon's project icon, a host-named favicon, avatar or upload, on the worktree list and the host workspace list; - PRCommentCard's author avatar, from the review reply schema; - the sealed HTML preview frame, which inherits the policy. Markdown and the editor are named as the anticipated surfaces ruling 26 points at, so a later reader does not take the loosening as already covering them. Both native pins carried the same wrong claim and are corrected. That comment is the only record of why the policy loosened, so it says what is true now and what is coming, separately. Comment only: the parsed header is unchanged, checked through the harness reader the render suite uses. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): point the new source-control route pin at the current directive Merge resolution, not a conflict git could see. #21957 landed the source-control and review page routes on main while this branch was open, and its render check pins the directive text twice: `cspHeader` by substring, which survives the widening, and the Swift source by the quoted literal `"img-src 'self' data:"`, which does not. Two PRs green alone, red on the merge. Both pins now read the current directive. One comment goes with it. "Not one request left the origin, so there is nothing for the policy to have refused" now needs saying why: `https:` is admitted, so an empty host list is these two closures fetching nothing rather than the policy refusing something. The avatar that would fetch needs provider data this page never gets, which the file's own closing note already explains. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): wait for the admitted images before reading their hits CI's Chrome 152 recorded the CSS background and not the `<img>` by the time the bounded settle returned, so both https arms failed on a count: "expected [ '/css-bg.png' ] to deeply equal [ '/css-bg.png', '/img.png' ]" and "expected 1 to be 2". The reads were absence-shaped -- two frames and 200 ms -- and the claim they carry is a presence. So the arms wait for their own evidence, the way the `'refusal'` arm already does. `frameReady: 'images'` polls until both admitted paths are recorded, bounded by nothing but the case's own `ctx.signal`. It sits after the marker wait, because an image is requested by a document that has parsed, and the arm hands its reader in rather than the settling module reaching for state that belongs to an arm. One reader now serves the wait and the reading. An arm that waits on one list and asserts on another has proved nothing about the list it asserts on. The `/probe.woff2` absence is untouched and is now an absence standing behind two presences rather than beside them. What the wait prints when it does not arrive, captured by making the paths unsatisfiable against a 12 s case: [html-preview-render] the arm recorded ["/img.png","/css-bg.png"] of ["/css-bg.png","/img.png","/never-arrives.png"]; #remote {"complete":true,"naturalWidth":1, "currentSrc":"https://artifact-images.invalid/img.png?n=n1", "loading":null}: arm csp=shipped sandbox=product frameReady=images nonce=n1 \| browser 147.0.7727.15 \| ... \| frames [...] `complete` with a zero `naturalWidth` is a request that finished and produced no image; `complete` false is one still in flight. So a Chrome that never issues the request says which of those it was, instead of a bare count. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): say why an admitted image never arrived, and hand back the context CI's Chrome 152 read the `<img>` as complete with a zero naturalWidth and a resolved currentSrc while the route handler never saw the request, and the CSS background from the same origin did reach it. The diagnosis could say the image failed but not why, because nothing was watching the request. Now four sources are, for the `.invalid` origin only, in a module of their own so the rig file stays under its cap: `request` says whether the page asked at all, `requestfailed` carries the browser's `errorText`, and CDP's `Network.loadingFailed` adds `blockedReason` and `corsErrorStatus`, which is the only place a refusal names itself once the request never reaches a route handler. `Network.requestWillBeSent` records the resource type, the initiator and the frame, which separates an image the parser found from one nothing asked for. They fill arrays while an arm passes and are only read on abort. Proved by forcing the abort rather than assuming: with the awaited paths made unsatisfiable, the reading names the font's refusal in both vocabularies at once, `failed [{"url":".../probe.woff2","errorText":"csp"}]` and `cdp loadingFailed [{"errorText":"","blockedReason":"csp",...,"type":"Font"}]`, beside `cdp sent` showing every request's type, initiator and frameId. Teardown: `open()` now takes an explicit context and closes both the page and the context in a `finally`. The close used to sit on the happy path, so an arm whose wait aborted and whose result reads then raced vitest's teardown left its page and its implicit context open on a browser every later case in that engine still runs on. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct three rationales the widening left wrong (a) A review comment's avatar is not a surface the widening unblocks. PRCommentCard renders it only under `Platform.OS !== 'web'` and a component test pins the skip, so on the page it never renders. Dropped from both native rationales and moved to the anticipated list beside markdown and the editor, with the reason each is anticipated rather than current. (b) The Kotlin rationale quoted the iOS origin. Android serves from `https://<sha256(sessionId) first 32 hex>.orca-mobile-web.invalid/`, so a referrer there carries a stable per-session handle and not the id itself, while iOS serves `orca-mobile-web://<sessionId>/` and carries it verbatim. Both are something an image host can key on across requests, which is what the header is for; each file now names its own origin. (c) "Only the script half of that is stricter than native" overstated it. `font-src 'none'` and `connect-src 'self'` are stricter too. Images are the one of the four that stopped being stricter, and the comment now says which three remain and why. A fourth, found while checking (a): the skip's own comment justified itself with `img-src` being `'self' data:`, so a provider avatar would be "one refused request per card". That is no longer true -- the avatar would load now -- so the skip is a page capability gap rather than a policy consequence. Recorded as such at the guard. Whether to lift the guard is a ruling-26 question and not this PR's. Comments only. The parsed policy and document headers are unchanged, checked through the harness readers the render suite uses. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): probe why Chrome never asks for the artifact image CI's read was decisive: on Chrome 152 only the CSS background was requested, while the `<img>` reported complete with a zero naturalWidth and a resolved currentSrc. A request that went out and failed cannot produce both readings, so the next probe asks the frame rather than the network. On abort it now reads, inside the artifact frame: readyState, the init script's own moment, document.images.length, every `performance.getEntriesByType('resource')` name, the navigation entry types, and for #remote its src, isConnected, complete, naturalWidth, currentSrc and the outcome of decode(). A resource entry for a URL the rig never saw would mean the request left the frame and died before reaching it. Then it issues a `new Image()` at a URL that has never existed and reports two seconds later whether the rig saw it. That splits the two live explanations: if the fresh request is seen and the artifact's was not, the frame can fetch and the parser-inserted element is the cause; if neither is seen, requests from this frame are not reaching the rig at all. Subframe document commits are counted from mount, because a second parse is a new window and leaves nothing behind to count, and a second parse could be meeting a failure the first cached. `cdp sent` was empty on CI even for a request Playwright did record, so the page's own session is blind to the frame. Chromium isolates sandboxed iframes into their own process, srcdoc included, so flattened Target.setAutoAttach now puts each child target on the same connection with Network.enable on the child, and the attached list reports whether the frame is a separate target at all. The navigation arm gets the same reading, since CI showed it fails on its own rather than behind the aborted image arms. Verified by forcing the abort rather than assumed. Locally the reading prints one subframe parse, decode resolved, every resource the document fetched, and `fresh ... issued true seen true`, with the attached list empty, which is consistent with this Chrome not isolating the frame and its page session seeing the requests. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): time the artifact image against the frame's attachment CI's second read showed the frame did issue the request -- it has a resource-timing entry and decode rejected with EncodingError -- while the rig saw only the CSS background, and a fresh image created later from the same frame was both issued and seen. The remaining question is whether the entry starts before anything was listening to that frame. So the entry is now reported in full for the element under test: responseStatus, transferSize, encodedBodySize, nextHopProtocol, startTime and duration. A zero status with a zero transferSize is a fetch that reached the network stack and came back with nothing, which is what an unintercepted request looks like once `.invalid` fails to resolve. Both sides of the comparison get a wall clock: `Target.attachedToTarget` and Playwright's own `frameattached` now carry the moment they fired, and every recorded request carries the moment it was seen. An entry that starts before the attachment is the race stated rather than inferred. Abort path only; the passing run is unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): serve the artifact's https assets from a real TLS listener Interception could not measure what the directive admits. Chrome 152 isolates the sandboxed srcdoc frame into its own target and the parser-inserted `<img>` is the document's first fetch, issued before interception attaches there: the request escaped to the real network, `artifact-images.invalid` did not resolve, and the rig recorded nothing while the frame's own resource timing showed the fetch and a later fresh image was both issued and seen. So the assets come from a listener that is already accepting before the page exists. It cannot be raced: the request arrives or it does not, and either answer is the measurement. Hits and referrers are recorded server-side, the way this rig's cleartext origin already does it, and read per arm by nonce. `img-src 'self' data: https:` matches on scheme, so `https://127.0.0.1:<port>` exercises the same directive as any other https host. Lifecycle: started in beforeAll before any browser, closed in afterAll beside the other servers. Its certificate is generated per run by openssl into the suite's own scratch directory under `mobile/.tmp`, which the root gitignore already covers and into which the server writes a second `.gitignore` as well; the key never leaves that directory and nothing trusts it, since the context is created with `ignoreHTTPSErrors`. No arm shares state: one hit list keyed by each arm's nonce, and the permissive-Referrer-Policy control stays what it was, a second bundle server serving the page, because the control is the document's header and not the image host's. The navigation record moves off interception too. It is now `page.on('request')`, one subscription over every frame, armed after the rig's own `goto` exactly where the route used to be registered; the route stays only for what only a route can do, refuse the navigation. That answers the top-nav arm's `recorded []`: its record depended on the same per-target interception. And the arms stop swallowing their clicks. `click(...).catch(() => {})` made a tap that never landed and a tap that produced no navigation the same empty counter; `open()` now records the error and the two top-nav arms assert it is null before reading any count. One correction to the reading added in the previous commit. The resource-timing fields came back zero for a request that had plainly succeeded: they are opaque cross-origin. The listener now sends `Timing-Allow-Origin`, after which transferSize, encodedBodySize and nextHopProtocol carry real values. `responseStatus` still reads zero on a successful request, so the comment names the three that discriminate rather than the four that are printed. 24/24 on both local engines. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): compare the artifact fetch and the attachment on one clock The early-or-late comparison spanned two clocks and could not answer the question it was written for. Every `at` in the request log is Node's `performance.now()`, counting from process start; the resource entry's `startTime` is the frame's own, counting from that document's navigation. A frame entry reads as earlier than a Node attachment by roughly the process uptime, so the comparison would have reported the race as confirmed on every run, including runs where there was no race. A green CI would not have caught it. So the comparison is stated where both numbers actually live: `asked` against `attached` in the request log, on the Node clock alone. `startTime` and `duration` stay, labelled as the frame's own account and explicitly not comparable to an attachment time. The module docstring says the same, so the next reading added here starts from the rule rather than rediscovering it. The commit message of `b5e82065f3` carries the same overstatement and is left as it stands; this is the correction. Also the stale route-handler references, now that the asset listener records the secure origin and the navigation record is a page subscription. Three were in the review; two more were not, and both were stale for the same reason: `waitForRecordedNavigation`'s docstring still credited the route with recording a main-frame navigation, which stopped being true when the record moved off interception, and the request log described a refusal as one the request never reached a route handler with. The route now only refuses; it counts nothing. The one remaining mention is the deliberate contrast in the rig that says the record is the page's event and not the route's. Comments only. 24/24 on both local engines. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-21 08:34:38 -04:00
Jinwoo Hong	23207bfde2	feat(mobile): register the source-control and review page routes (OTA phase C, C4.4) (#21957 ) * refactor(mobile): move the review route body onto a component and the handoff seam (OTA phase C, C4.4) The review route file called `useMobileDiffReviewController` at its top level. A switch cannot keep it there: hooks are unconditional, so the whole controller — its client subscriptions included — would run behind the shell's page whenever the shell renders. As an element passed for `fallback` it is created and not mounted, which is how the explorer switch already behaves. `useRouter` becomes `useRouteHandoff` in the same move. It was the one raw expo-router router left in the review closure (measured: the only other value import of one is the seam's own web sibling), and inside the page the session screen `openSession` replaces to is native, so that target has to be handed back to the app rather than posted into a document that does not render it. The params are read in the component rather than handed down, so this is the route body and the route file above it is free to become a switch. `session-router-seam-census.test.ts` gains the module by name. Kept with `useRouter` the census reds twice — `imports nothing from expo-router that can navigate` names `MobileDiffReviewRouteScreen.tsx (useRouter)`, and the completeness case gains `useRouter` — which is what forces the swap rather than leaving it to a reviewer. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): switch the source-control and review routes to the shell, still unregistered (OTA phase C, C4.4) Both take the files switch's shape: `firstParam`/`firstReviewParam` on every param, `shellScreenRoute` as the one predicate, `MobileWebShellScreen` keyed on `shellScreenRouteKey`, the native screen built as an element and passed for `fallback`. Both gain a `.web.tsx` sibling for `index.web.tsx`'s reason — the native file reaches OrcaMobileWebShellView, whose module throws at import in a browser, and the route manifest imports every route. Inert on its own. A switched route renders the shell only once `MOBILE_WEB_PAGE_ROUTES` lists it, which is the next commit; until then the flag is the only thing that changes and it is off. Query params are omitted rather than sent empty, and the whole record is omitted when none was named: `tab=` is a lens named nothing and lands on `changes` through a different branch than an absent one, and the same holds for `name`, `origin`, `scope`, `file` and `area`. `pr` and `history` are deliberately not switched. Both are `Redirect`s into `source-control`, and a redirect inside the page would leave the session bound to a pathname the page has left; left native they replace into this route and its switch mounts the shell. Three censuses red without their rows, measured on this tree: - `mobile-web-app-web-overrides.test.mjs` `lists exactly the .web.* files on disk` names the two new siblings; `states a reason for every override` reds on a placeholder under 20 characters. - `mobile-web-shell-flag-census.test.ts` `reaches the switched routes through that hook and no others` reds without the two `SWITCHED_ROUTES` names. - `shell-screen-route-census.test.ts` `walks the route tree and finds them` reds without the two switch names. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): register the source-control and review page routes (OTA phase C, C4.4) Two entries in `MOBILE_WEB_PAGE_ROUTES`, five grants each, with the reason for each grant read off the screen that needs it. The two lists are equal on purpose: the hub's changed-file rows push review and review replaces back, and a target declaring no more than its opener is a hop the handoff keeps inside the document. Registering either alone would have put a native frame and a second bridge session between a changed-file row and its diff. `pageRouteGrants` is derived from this list, so the two rows are a consequence of the entries and there is no second table to edit. `pr` and `history` stay native redirects and are never listed; the derived target list at this tree is [files, files/preview, source-control, [p], accounts, agent-history, review, session, tasks, web], with no `pr` or `history` row, because the census reads call sites and both redirects name `source-control`. Measured on this tree, not carried from the draft: - The hop census goes 8 -> 16. The eight new rows are exactly `{/h/[hostId], agent-history, files/[worktreeId], files/preview} -> {source-control, review}`, each handed off for `native.clipboard.write` and the first four also for `externalLink`. `source-control <-> review` is absent in both directions, which a new case now asserts as grant-list equality rather than as the absence of a row — absent is also what an unregistered route looks like. - The Back census now walks six trees and finds 8 controls, both rules printing empty. The two new ones are `MobileSourceControlHeader.tsx:46 role=button label=Back to session` and `MobileDiffReviewHeader.tsx:48 role=button label=Back`, which is what C4.3 bought. The `ARRIVING_SCREENS` describe it wrote for this moment is removed: with the rows in `PAGE_SERVED_SCREENS` its trees are covered and its cases were a second reading of the same thing. - Both closures reach the haptics seam, so `haptics` is declared by measurement: the seam census derives the reaching set and its two cases pass with the routes in its `ROUTE_MODULES` map. Without the two manifest entries these red on this tree: `pins every hop the handoff must take away from the page`, `keeps the hub and review local to each other`, `declares only routes the bundle has a module for`, `reaches the built manifest`, `covers every page route and finds a control in each`, and both haptics-seam cases. `build-mobile-web-app-bundle.test.mjs` is split rather than fenced. The two pinned entries put it at 607 non-comment lines against the 600 cap, and the declaration block is a different concern from how the bundle is built — it grows once per registered domain while that file does not. It moves whole into `mobile-web-page-routes.test.mjs`, named for the module it is written against, so the next route to register does not have to choose between a lint fence and a split it did not ask for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): render-check the two page routes, and make the oversized stage-all readable (OTA phase C, C4.4) The render check mounts both routes in a real browser, asserts each paints its own screen rather than the Unmatched route with no console error and no page fault, and asserts each fetches its own chunk on a client-side navigation. It also reads the shipped `img-src 'self' data:` out of the Kotlin source it is served with, pins the Swift twin beside it, and asserts neither route leaves the origin or logs a policy violation while it paints. The avatar skip itself (ruling 3) is `PRCommentCard`: on web it renders its existing empty-avatar `View` rather than letting one `<Image>` per comment attempt a fetch the policy refuses. Its branch is pinned by a component test, which reds on the platform check being removed. The render check's off-origin case is honest about being the negative half only — no comment card renders there, because the PR chain behind it is not scripted, and the file says so. The `useAnimatedScrollHandler` risk is answered by the two static facts rather than by a probe, and they are recorded as assertions: the hook is deliberately outside the four `MAPPER_HOOKS` because it is an event handler, and its updater's only effect is a write to `scrollOffsetY`, which `RightDrawer.tsx` assigns in two places and reads in none. A later read reds that case the moment it is added. The `oversized` stage-all refusal (ruling 2, made testable by ruling 5) was a silent no-op, and this is the fix as well as the case. Measured on this tree before it: `git.bulkStage` with 12,000 paths posts one 1,033,012-byte frame, the shell's reader drops it with `{ kind: 'refused', refusal: 'oversized' }`, and the page's promise never settles — `busyAction` never cleared and `setActionError` was never called. Both new cases red by timing out at 15s against that path. Refused at the page's own send boundary instead, under the shell reader's own predicate rather than a second spelling of it: `isBridgeFrameWithinCap` is extracted from `parseBridgeMessage` and used by both sides. `sendFrame` answers `sent` / `oversized` / `port-failed`, so `sendRequest` rejects with a `BridgeRequestOversizedError` whose message is a sentence the panel puts on its error surface, and the members whose contract is a boolean keep it. No delivery-unknown mark: the frame never left, so nothing ran on the desktop and the smaller retry is safe to offer. The case runs the real chain — bridge port pair, `useMobileGitRequests`, `runGitWorkflow` — with only react-native and the haptics seam mocked, and asserts the message that lands is a sentence and that the busy flag is raised and then cleared. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): drop the type assertions from the two new C4.4 test files (OTA phase C, C4.4) The changed-code quality gate named five, all in the files the previous commit added, and a fence is not the answer to any of them. A separate commit because a reported head does not move by amend. - The comment fixture is a real `PRComment` rather than a cast: the type's six required fields are all this case needs, and the SAFETY disable that stood in for them was inert anyway — oxfmt had wrapped it onto three lines, and a wrapped `oxlint-disable-next-line` matches nothing. - The image lookup goes through `findAll` on the host tag rather than `findAllByType`, which takes a component. Through `String`, because `node.type` is `ElementType` and React Native declares no intrinsic elements, so the compiler reads a bare tag comparison as unreachable. - The runners hook takes its router from `useRouteHandoff` with expo-router mocked under it, which is how a `RouteHandoff` is obtained rather than asserted into existence. No target is pressed. - The rejection and the diagnostic are read through narrowings instead of casts, which also drops an `expect.any` that only type-checked because of one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep a frame the page cannot serialize inside the send contract (OTA phase C, C4.4 round 1) Round 1 finding 1. The oversized refusal moved `JSON.stringify` outside `sendFrame`'s `try`, so a frame carrying a cycle, a `BigInt` or a throwing `toJSON` threw past the whole send path. Three things followed, all measured here on a cyclic `params`: - the caller was rejected with a bare `TypeError` from `JSON.stringify` instead of the `BridgeSendFailedError` every other undelivered frame raises; - no `send-failed` diagnostic was raised, so nothing recorded that a frame had been lost; - `sendRequest` opens the id before it posts and abandons it on the way out, and the throw skipped the abandon: 63 of the 64 in-flight slots were usable afterwards, against 64 on a client that sent no such frame. Sixty-four of them and every later request is refused with nothing to say why. `posted()` carried the same escape into the members whose contract is a boolean, where a throw is worse still: those callers are taps and teardowns with no catch on them. Serialization goes back inside the `try`, with the oversized refusal kept in front of the post. The docstring said the port arm's throw is never `JSON.stringify`'s, which was exactly the assumption that broke; it now says why the call sits where it does. The new file is the pin: the rejection's name, the diagnostic, nothing reaching the shell, and the slot count with a no-cyclic-frame control beside it so the count cannot pass by the cap moving. Both changed cases red on the serialization moving back out — `expected 'TypeError' to be 'BridgeSendFailedError'` and `expected 63 to be 64`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the page's frame cap to the reader's, at the boundary and by construction (OTA phase C, C4.4 round 1) Round 1 finding 2. Nothing held the sender's predicate to the reader's. Replacing `isBridgeFrameWithinCap(json)` with an inline `json.length > BRIDGE_MAX_MESSAGE_BYTES + 1` passed 85 of the 86 mobile-web-shell and source-control test files on this tree, and a frame at exactly cap+1 would then be posted and silently dropped — the hang the refusal exists to end, back for every frame in that one-unit band. Two rules, because either alone passes against the defect: - The boundary. A frame of exactly the cap is posted, arrives at `parseBridgeMessage` and is accepted; a frame one byte over is refused with `BridgeRequestOversizedError`, posts nothing, and is the same string the reader answers `oversized` to. An off-by-one reds the second. - The census. The client reaches the cap through the shared predicate and does not name `BRIDGE_MAX_MESSAGE_BYTES` at all, and the module that exports the predicate is the module that parses inbound frames. A private copy that is correct on the day it is written reds here. The overhead the boundary frames are built from is itself checked rather than trusted: a frame asked for at exactly the cap must serialize to exactly the cap, so the constant cannot rot behind an envelope that grew a field. Against the mutation both new rules red — `expected null to be 'BridgeRequestOversizedError'` and the census failing to find the predicate — while the rest of the suite stays green, which is the finding reproduced. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): count the outbound frame in the unit both shells count it in (OTA phase C, C4.4 round 1) Round 1 finding 3. Read off both shells rather than assumed, and they agree: iOS gates the inbound frame on `json.utf8.count` (`MobileWebShellView.swift`, through `MobileWebShellBridge.acceptsByteCount`) and Android on `json.toByteArray(Charsets.UTF_8).size` (`MobileWebShellView.kt`, through `acceptsMobileWebShellBridgeByteCount`), both against `640 * 1024`. UTF-8 bytes on each platform. The predicate was already right. `isBridgeFrameWithinCap` decides on `utf8ByteLength`, and the `raw.length` clause in front of it is a cheap refusal in the safe direction, not a second rule: every code unit encodes to at least one byte, so a string over the cap in units is over it in bytes too. The diagnostic was not. It reported `json.length` — UTF-16 code units — in a field named `bytes`, so a frame of CJK text read as a quarter of the cap at the moment it was refused by it. It now reports `utf8ByteLength(json)`, and the type says which unit that is. Pinned with a 250,000-character frame of three-byte characters, which is under the cap in code units and over it in bytes, plus a source case reading the measuring expression out of each shell. Three mutations, all red: dropping the byte clause from the predicate reds the refusal (`expected null to be 'BridgeRequestOversizedError'`) and the diagnostic; reporting `json.length` again reds the diagnostic alone (`expected 250094 to be greater than 655360`), which is the defect this commit fixes, in the number it would have printed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(config): drop the render check's avatar assertion, which could not fail (OTA phase C, C4.4 round 1) Round 1 finding 4. The case asserted that no avatar host was requested while both routes painted, which reads as a proof of the web skip and is not one: no comment card renders on either page, because the PR chain the file's own closing note names is not scripted. Reproduced here — deleting the `Platform.OS !== 'web'` guard from `PRCommentCard` leaves the file at 5 passed. Deleted rather than propped up. Giving the page a presence precondition means five hand-written fixtures against five Zod schemas inside the shell double, which is exactly what the harness's docstring says that double must not become. So the only proof of that branch is `pr-comment-card-web-avatar.test.tsx`, which reds when the check is removed, and the render check now says so in its header instead of implying otherwise. What survives is a property of these two closures rather than of that component: not one request leaves the origin while either route paints, and nothing either paints violates the policy. That one can fail — planting a `fetch` to a provider host in a module both routes reach reds it twice, on the console-error case and on the off-origin case, with the `connect-src 'self'` refusal in the output. The CSP half is unchanged and was never in question: the served header is read from the Kotlin source and the Swift twin is pinned beside it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-21 05:32:24 -04:00

Jinwoo Hong

a3c6d4266a

fix(mobile): admit https: images on the web shell's CSP (OTA phase C, ruling 27) (#21964 )

* fix(mobile): admit https: images on the web shell's CSP (OTA phase C, ruling 27)

Native markdown and the native rich editor load images the author referenced
by URL, so the page has to as well or a remote image is a blank where native
paints a picture. `img-src` widens to `img-src 'self' data: https:` on both
platforms; `script-src`, `connect-src`, `object-src`, `frame-src` and
`child-src` do not move.

`http:` stays out, and the pins say so directly rather than by absence: the
Kotlin test's blanket `!contains("http")` could not survive `https:`, so both
native pins now check `http:` (not a substring of `https:`) and check that
`https:` appears in `img-src` and nowhere else, the same shape the `data:`
pin already had.

No behaviour change on released phones: the shell ships in no released tag
(mobile-v0.0.9 predates it), so this reaches devices with the Phase E native
build and not before.

Neither native module has a CI job, so both ran locally: swiftc over the
module plus MobileWebShellChecks, and
`:orca-mobile-web-shell:testDebugUnitTest`. Both were confirmed red against
the old directive first.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(mobile): correct what the sealed preview frame is stricter about

The doc comment said the page was deliberately stricter than the native
preview because it loads no remote image and runs no script. Since `img-src`
gained `https:` only the script half is true: the frame loads a remote image
exactly as the native WebView does.

Says instead what an artifact's image URL now is -- a channel that fires on
view and carries whatever its author encoded, with nothing dynamic behind it
because no script runs -- and names `referrerPolicy` as what keeps the
document's own origin out of the request.

Comment only; no behaviour and no test moves.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): measure both halves of the preview frame's image fence

"fetches nothing of the artifact that leaves the origin" stopped being what
the sealed arm proves once `img-src` gained `https:`. The fixture's foreign
origin is `http://127.0.0.1`, so its two images are refused on the scheme
alone and only the font is refused by `font-src 'none'`. Renamed to say
exactly that.

The half that was missing is an https arm. Playwright route interception
answers an `https://…invalid` origin in the page, so the arm needs no TLS
server and no new dependency, and a request only reaches the handler if the
policy let it out. Under the shipped header, on Chromium and WebKit, the
`<img>` and the CSS background are both requested -- `img-src` governs a
background too -- and the font still is not.

`artifact()` takes the subresource origin; the links stay on the cleartext
one so no existing navigation case changes.

Red-first: with `img-src 'self' data:` put back into the parsed Kotlin
policy, the new arm fails on both engines with `expected [] to deeply equal
[ '/css-bg.png', '/img.png' ]`. The directive was restored byte-identical
before this commit.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* refactor(scripts): split the preview frame's settling out of the render check

The https arm pushed mobile-web-app-html-preview-render.test.mjs to 620
counted lines, over the 600 cap config/scripts carries. Split at a module
boundary rather than bumped: the four wait-and-settle functions are rig
mechanics with no assertion in them, and they now sit beside the diagnosis
module they already reported through.

`waitForLoadedFrame` and `settleAfterMount` are the two the render check
calls; `waitForRecordedNavigation` and `settleWithoutNavigation` stay
internal to the new module.

Move only. Same 20 tests pass on both engines.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): send Referrer-Policy: no-referrer on the shell document

`img-src https:` gave the page somewhere to send a request, and the document
origin is `orca-mobile-web://<sessionId>/`, so a request that carries a
referrer carries the session id to whatever host an artifact or a markdown
document named.

`referrerPolicy="no-referrer"` on the preview iframe does not cover it.
Measured in the render rig against a permissive control policy: WebKit puts
the embedder's URL on a srcdoc frame's image request despite the attribute,
and Chromium sends none. So the guarantee belongs on the document, where one
header covers every request the page makes, and it rides the document alone
with the policy -- the referrer of a request is decided by the document that
made it, so on a subresource response it would govern nothing.

WKWebView under the custom scheme is unverified: the rig is Playwright
WebKit over http, not WKWebView over `orca-mobile-web://`. The header is the
hedge, and it costs nothing if that host never leaked.

Pinned three ways, each confirmed red first:
- Swift, exit 133 with the header removed.
- Kotlin, MobileWebShellResponseHeadersTest "sends the policy on the
  document" FAILED at :17 with it removed.
- The rig, through a new `readShellDocumentHeaders` that parses the Kotlin
  source the way `readShellCsp` does and throws rather than returning an
  empty map. With the value flipped to `unsafe-url` the WebKit arm fails
  `expected [ …(2) ] to deeply equal [ null, null ]`; with the line deleted
  the parse throws "could not parse the shell document headers".

The rig's arm carries its own presence precondition: a third server serves
the shipped policy with `unsafe-url`, so the WebKit reading is the header
doing the work, and Chromium's null either way is pinned as the browser's
behaviour rather than sold as evidence the header arrived.

MobileHtmlPreview.web.tsx said the iframe attribute kept the origin out of
the request. Corrected to name the header, since the measurement above is
what disproved it.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(mobile): quote the current directive where the old text was written down

Three comments still read `img-src 'self' data:`, so a grep for the old
directive found live prose that no longer matches the header. Each stays
about `data:`, which is what those paths rest on; only the quoted policy
changes.

The two remaining hits in the repo are src/main/browser/doc-preview-protocol,
which is the desktop preview's own policy and not this one.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(mobile): name the surfaces img-src https: actually unblocks today

The comment justified `https:` with markdown and the rich editor, and
neither renders a remote image on the page. Verified in the tree:
MobileMarkdown paints `![](...)` as a tappable link at both of its image
branches and never mounts an Image, and it has no `.web` sibling, so that is
what native does too; MobileRichMarkdownEditor.web.tsx is a 92-line
multiline TextInput, still C7.6's plain source field.

What the directive unblocks today is four surfaces, none of them overridden
on the page:
- MobileAgentIcon's favicon, a hardcoded `google.com/s2/favicons` URL, used
  by thirteen callers including the session header and the worktree rows;
- MobileRepoIcon's project icon, a host-named favicon, avatar or upload, on
  the worktree list and the host workspace list;
- PRCommentCard's author avatar, from the review reply schema;
- the sealed HTML preview frame, which inherits the policy.

Markdown and the editor are named as the anticipated surfaces ruling 26
points at, so a later reader does not take the loosening as already covering
them. Both native pins carried the same wrong claim and are corrected.

That comment is the only record of why the policy loosened, so it says what
is true now and what is coming, separately.

Comment only: the parsed header is unchanged, checked through the harness
reader the render suite uses.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): point the new source-control route pin at the current directive

Merge resolution, not a conflict git could see. #21957 landed the
source-control and review page routes on main while this branch was open,
and its render check pins the directive text twice: `cspHeader` by substring,
which survives the widening, and the Swift source by the quoted literal
`"img-src 'self' data:"`, which does not. Two PRs green alone, red on the
merge.

Both pins now read the current directive.

One comment goes with it. "Not one request left the origin, so there is
nothing for the policy to have refused" now needs saying why: `https:` is
admitted, so an empty host list is these two closures fetching nothing
rather than the policy refusing something. The avatar that would fetch needs
provider data this page never gets, which the file's own closing note
already explains.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): wait for the admitted images before reading their hits

CI's Chrome 152 recorded the CSS background and not the `<img>` by the time
the bounded settle returned, so both https arms failed on a count: "expected
[ '/css-bg.png' ] to deeply equal [ '/css-bg.png', '/img.png' ]" and
"expected 1 to be 2". The reads were absence-shaped -- two frames and 200 ms
-- and the claim they carry is a presence.

So the arms wait for their own evidence, the way the `'refusal'` arm already
does. `frameReady: 'images'` polls until both admitted paths are recorded,
bounded by nothing but the case's own `ctx.signal`. It sits after the marker
wait, because an image is requested by a document that has parsed, and the
arm hands its reader in rather than the settling module reaching for state
that belongs to an arm.

One reader now serves the wait and the reading. An arm that waits on one
list and asserts on another has proved nothing about the list it asserts on.

The `/probe.woff2` absence is untouched and is now an absence standing
behind two presences rather than beside them.

What the wait prints when it does not arrive, captured by making the paths
unsatisfiable against a 12 s case:

  [html-preview-render] the arm recorded ["/img.png","/css-bg.png"] of
  ["/css-bg.png","/img.png","/never-arrives.png"]; #remote
  {"complete":true,"naturalWidth":1,
  "currentSrc":"https://artifact-images.invalid/img.png?n=n1",
  "loading":null}: arm csp=shipped sandbox=product frameReady=images
  nonce=n1 | browser 147.0.7727.15 | ... | frames [...]

`complete` with a zero `naturalWidth` is a request that finished and
produced no image; `complete` false is one still in flight. So a Chrome that
never issues the request says which of those it was, instead of a bare count.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): say why an admitted image never arrived, and hand back the context

CI's Chrome 152 read the `<img>` as complete with a zero naturalWidth and a
resolved currentSrc while the route handler never saw the request, and the
CSS background from the same origin did reach it. The diagnosis could say
the image failed but not why, because nothing was watching the request.

Now four sources are, for the `.invalid` origin only, in a module of their
own so the rig file stays under its cap: `request` says whether the page
asked at all, `requestfailed` carries the browser's `errorText`, and CDP's
`Network.loadingFailed` adds `blockedReason` and `corsErrorStatus`, which is
the only place a refusal names itself once the request never reaches a route
handler. `Network.requestWillBeSent` records the resource type, the initiator
and the frame, which separates an image the parser found from one nothing
asked for. They fill arrays while an arm passes and are only read on abort.

Proved by forcing the abort rather than assuming: with the awaited paths made
unsatisfiable, the reading names the font's refusal in both vocabularies at
once, `failed [{"url":".../probe.woff2","errorText":"csp"}]` and `cdp
loadingFailed [{"errorText":"","blockedReason":"csp",...,"type":"Font"}]`,
beside `cdp sent` showing every request's type, initiator and frameId.

Teardown: `open()` now takes an explicit context and closes both the page and
the context in a `finally`. The close used to sit on the happy path, so an
arm whose wait aborted and whose result reads then raced vitest's teardown
left its page and its implicit context open on a browser every later case in
that engine still runs on.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(mobile): correct three rationales the widening left wrong

(a) A review comment's avatar is not a surface the widening unblocks.
PRCommentCard renders it only under `Platform.OS !== 'web'` and a component
test pins the skip, so on the page it never renders. Dropped from both native
rationales and moved to the anticipated list beside markdown and the editor,
with the reason each is anticipated rather than current.

(b) The Kotlin rationale quoted the iOS origin. Android serves from
`https://<sha256(sessionId) first 32 hex>.orca-mobile-web.invalid/`, so a
referrer there carries a stable per-session handle and not the id itself,
while iOS serves `orca-mobile-web://<sessionId>/` and carries it verbatim.
Both are something an image host can key on across requests, which is what
the header is for; each file now names its own origin.

(c) "Only the script half of that is stricter than native" overstated it.
`font-src 'none'` and `connect-src 'self'` are stricter too. Images are the
one of the four that stopped being stricter, and the comment now says which
three remain and why.

A fourth, found while checking (a): the skip's own comment justified itself
with `img-src` being `'self' data:`, so a provider avatar would be "one
refused request per card". That is no longer true -- the avatar would load
now -- so the skip is a page capability gap rather than a policy consequence.
Recorded as such at the guard. Whether to lift the guard is a ruling-26
question and not this PR's.

Comments only. The parsed policy and document headers are unchanged, checked
through the harness readers the render suite uses.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): probe why Chrome never asks for the artifact image

CI's read was decisive: on Chrome 152 only the CSS background was requested,
while the `<img>` reported complete with a zero naturalWidth and a resolved
currentSrc. A request that went out and failed cannot produce both readings,
so the next probe asks the frame rather than the network.

On abort it now reads, inside the artifact frame: readyState, the init
script's own moment, document.images.length, every
`performance.getEntriesByType('resource')` name, the navigation entry types,
and for #remote its src, isConnected, complete, naturalWidth, currentSrc and
the outcome of decode(). A resource entry for a URL the rig never saw would
mean the request left the frame and died before reaching it.

Then it issues a `new Image()` at a URL that has never existed and reports two
seconds later whether the rig saw it. That splits the two live explanations: if
the fresh request is seen and the artifact's was not, the frame can fetch and
the parser-inserted element is the cause; if neither is seen, requests from
this frame are not reaching the rig at all. Subframe document commits are
counted from mount, because a second parse is a new window and leaves nothing
behind to count, and a second parse could be meeting a failure the first
cached.

`cdp sent` was empty on CI even for a request Playwright did record, so the
page's own session is blind to the frame. Chromium isolates sandboxed iframes
into their own process, srcdoc included, so flattened Target.setAutoAttach now
puts each child target on the same connection with Network.enable on the
child, and the attached list reports whether the frame is a separate target
at all.

The navigation arm gets the same reading, since CI showed it fails on its own
rather than behind the aborted image arms.

Verified by forcing the abort rather than assumed. Locally the reading prints
one subframe parse, decode resolved, every resource the document fetched, and
`fresh ... issued true seen true`, with the attached list empty, which is
consistent with this Chrome not isolating the frame and its page session
seeing the requests.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): time the artifact image against the frame's attachment

CI's second read showed the frame did issue the request -- it has a
resource-timing entry and decode rejected with EncodingError -- while the rig
saw only the CSS background, and a fresh image created later from the same
frame was both issued and seen. The remaining question is whether the entry
starts before anything was listening to that frame.

So the entry is now reported in full for the element under test:
responseStatus, transferSize, encodedBodySize, nextHopProtocol, startTime and
duration. A zero status with a zero transferSize is a fetch that reached the
network stack and came back with nothing, which is what an unintercepted
request looks like once `.invalid` fails to resolve.

Both sides of the comparison get a wall clock: `Target.attachedToTarget` and
Playwright's own `frameattached` now carry the moment they fired, and every
recorded request carries the moment it was seen. An entry that starts before
the attachment is the race stated rather than inferred.

Abort path only; the passing run is unchanged.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): serve the artifact's https assets from a real TLS listener

Interception could not measure what the directive admits. Chrome 152 isolates
the sandboxed srcdoc frame into its own target and the parser-inserted `<img>`
is the document's first fetch, issued before interception attaches there: the
request escaped to the real network, `artifact-images.invalid` did not
resolve, and the rig recorded nothing while the frame's own resource timing
showed the fetch and a later fresh image was both issued and seen.

So the assets come from a listener that is already accepting before the page
exists. It cannot be raced: the request arrives or it does not, and either
answer is the measurement. Hits and referrers are recorded server-side, the
way this rig's cleartext origin already does it, and read per arm by nonce.
`img-src 'self' data: https:` matches on scheme, so `https://127.0.0.1:<port>`
exercises the same directive as any other https host.

Lifecycle: started in beforeAll before any browser, closed in afterAll beside
the other servers. Its certificate is generated per run by openssl into the
suite's own scratch directory under `mobile/.tmp`, which the root gitignore
already covers and into which the server writes a second `.gitignore` as well;
the key never leaves that directory and nothing trusts it, since the context
is created with `ignoreHTTPSErrors`. No arm shares state: one hit list keyed
by each arm's nonce, and the permissive-Referrer-Policy control stays what it
was, a second bundle server serving the page, because the control is the
document's header and not the image host's.

The navigation record moves off interception too. It is now `page.on('request')`,
one subscription over every frame, armed after the rig's own `goto` exactly
where the route used to be registered; the route stays only for what only a
route can do, refuse the navigation. That answers the top-nav arm's `recorded
[]`: its record depended on the same per-target interception.

And the arms stop swallowing their clicks. `click(...).catch(() => {})` made a
tap that never landed and a tap that produced no navigation the same empty
counter; `open()` now records the error and the two top-nav arms assert it is
null before reading any count.

One correction to the reading added in the previous commit. The resource-timing
fields came back zero for a request that had plainly succeeded: they are opaque
cross-origin. The listener now sends `Timing-Allow-Origin`, after which
transferSize, encodedBodySize and nextHopProtocol carry real values.
`responseStatus` still reads zero on a successful request, so the comment names
the three that discriminate rather than the four that are printed.

24/24 on both local engines.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): compare the artifact fetch and the attachment on one clock

The early-or-late comparison spanned two clocks and could not answer the
question it was written for. Every `at` in the request log is Node's
`performance.now()`, counting from process start; the resource entry's
`startTime` is the frame's own, counting from that document's navigation. A
frame entry reads as earlier than a Node attachment by roughly the process
uptime, so the comparison would have reported the race as confirmed on every
run, including runs where there was no race. A green CI would not have caught
it.

So the comparison is stated where both numbers actually live: `asked` against
`attached` in the request log, on the Node clock alone. `startTime` and
`duration` stay, labelled as the frame's own account and explicitly not
comparable to an attachment time. The module docstring says the same, so the
next reading added here starts from the rule rather than rediscovering it.

The commit message of b5e82065f3 carries the same overstatement and is left
as it stands; this is the correction.

Also the stale route-handler references, now that the asset listener records
the secure origin and the navigation record is a page subscription. Three were
in the review; two more were not, and both were stale for the same reason:
`waitForRecordedNavigation`'s docstring still credited the route with
recording a main-frame navigation, which stopped being true when the record
moved off interception, and the request log described a refusal as one the
request never reached a route handler with. The route now only refuses; it
counts nothing. The one remaining mention is the deliberate contrast in the
rig that says the record is the page's event and not the route's.

Comments only. 24/24 on both local engines.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

2026-09-21 08:34:38 -04:00

Jinwoo Hong

23207bfde2

feat(mobile): register the source-control and review page routes (OTA phase C, C4.4) (#21957 )

* refactor(mobile): move the review route body onto a component and the handoff seam (OTA phase C, C4.4)

The review route file called `useMobileDiffReviewController` at its top level. A switch cannot
keep it there: hooks are unconditional, so the whole controller — its client subscriptions
included — would run behind the shell's page whenever the shell renders. As an element passed for
`fallback` it is created and not mounted, which is how the explorer switch already behaves.

`useRouter` becomes `useRouteHandoff` in the same move. It was the one raw expo-router router left
in the review closure (measured: the only other value import of one is the seam's own web sibling),
and inside the page the session screen `openSession` replaces to is native, so that target has to
be handed back to the app rather than posted into a document that does not render it.

The params are read in the component rather than handed down, so this is the route body and the
route file above it is free to become a switch.

`session-router-seam-census.test.ts` gains the module by name. Kept with `useRouter` the census
reds twice — `imports nothing from expo-router that can navigate` names
`MobileDiffReviewRouteScreen.tsx (useRouter)`, and the completeness case gains `useRouter` — which
is what forces the swap rather than leaving it to a reviewer.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* feat(mobile): switch the source-control and review routes to the shell, still unregistered (OTA phase C, C4.4)

Both take the files switch's shape: `firstParam`/`firstReviewParam` on every param, `shellScreenRoute`
as the one predicate, `MobileWebShellScreen` keyed on `shellScreenRouteKey`, the native screen built
as an element and passed for `fallback`. Both gain a `.web.tsx` sibling for `index.web.tsx`'s reason —
the native file reaches OrcaMobileWebShellView, whose module throws at import in a browser, and the
route manifest imports every route.

Inert on its own. A switched route renders the shell only once `MOBILE_WEB_PAGE_ROUTES` lists it,
which is the next commit; until then the flag is the only thing that changes and it is off.

Query params are omitted rather than sent empty, and the whole record is omitted when none was
named: `tab=` is a lens named nothing and lands on `changes` through a different branch than an
absent one, and the same holds for `name`, `origin`, `scope`, `file` and `area`.

`pr` and `history` are deliberately not switched. Both are `Redirect`s into `source-control`, and a
redirect inside the page would leave the session bound to a pathname the page has left; left native
they replace into this route and its switch mounts the shell.

Three censuses red without their rows, measured on this tree:
- `mobile-web-app-web-overrides.test.mjs` `lists exactly the .web.* files on disk` names the two new
siblings; `states a reason for every override` reds on a placeholder under 20 characters.
- `mobile-web-shell-flag-census.test.ts` `reaches the switched routes through that hook and no
others` reds without the two `SWITCHED_ROUTES` names.
- `shell-screen-route-census.test.ts` `walks the route tree and finds them` reds without the two
switch names.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* feat(mobile): register the source-control and review page routes (OTA phase C, C4.4)

Two entries in `MOBILE_WEB_PAGE_ROUTES`, five grants each, with the reason for each grant read off
the screen that needs it. The two lists are equal on purpose: the hub's changed-file rows push
review and review replaces back, and a target declaring no more than its opener is a hop the
handoff keeps inside the document. Registering either alone would have put a native frame and a
second bridge session between a changed-file row and its diff.

`pageRouteGrants` is derived from this list, so the two rows are a consequence of the entries and
there is no second table to edit. `pr` and `history` stay native redirects and are never listed; the
derived target list at this tree is [files, files/preview, source-control, [p], accounts,
agent-history, review, session, tasks, web], with no `pr` or `history` row, because the census reads
call sites and both redirects name `source-control`.

Measured on this tree, not carried from the draft:
- The hop census goes 8 -> 16. The eight new rows are exactly `{/h/[hostId], agent-history,
files/[worktreeId], files/preview} -> {source-control, review}`, each handed off for
`native.clipboard.write` and the first four also for `externalLink`. `source-control <-> review`
is absent in both directions, which a new case now asserts as grant-list equality rather than as
the absence of a row — absent is also what an unregistered route looks like.
- The Back census now walks six trees and finds 8 controls, both rules printing empty. The two new
ones are `MobileSourceControlHeader.tsx:46 role=button label=Back to session` and
`MobileDiffReviewHeader.tsx:48 role=button label=Back`, which is what C4.3 bought. The
`ARRIVING_SCREENS` describe it wrote for this moment is removed: with the rows in
`PAGE_SERVED_SCREENS` its trees are covered and its cases were a second reading of the same thing.
- Both closures reach the haptics seam, so `haptics` is declared by measurement: the seam census
derives the reaching set and its two cases pass with the routes in its `ROUTE_MODULES` map.

Without the two manifest entries these red on this tree: `pins every hop the handoff must take away
from the page`, `keeps the hub and review local to each other`, `declares only routes the bundle has
a module for`, `reaches the built manifest`, `covers every page route and finds a control in each`,
and both haptics-seam cases.

`build-mobile-web-app-bundle.test.mjs` is split rather than fenced. The two pinned entries put it at
607 non-comment lines against the 600 cap, and the declaration block is a different concern from how
the bundle is built — it grows once per registered domain while that file does not. It moves whole
into `mobile-web-page-routes.test.mjs`, named for the module it is written against, so the next route
to register does not have to choose between a lint fence and a split it did not ask for.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): render-check the two page routes, and make the oversized stage-all readable (OTA phase C, C4.4)

The render check mounts both routes in a real browser, asserts each paints its own screen rather
than the Unmatched route with no console error and no page fault, and asserts each fetches its own
chunk on a client-side navigation. It also reads the shipped `img-src 'self' data:` out of the
Kotlin source it is served with, pins the Swift twin beside it, and asserts neither route leaves the
origin or logs a policy violation while it paints.

The avatar skip itself (ruling 3) is `PRCommentCard`: on web it renders its existing empty-avatar
`View` rather than letting one `<Image>` per comment attempt a fetch the policy refuses. Its branch
is pinned by a component test, which reds on the platform check being removed. The render check's
off-origin case is honest about being the negative half only — no comment card renders there,
because the PR chain behind it is not scripted, and the file says so.

The `useAnimatedScrollHandler` risk is answered by the two static facts rather than by a probe, and
they are recorded as assertions: the hook is deliberately outside the four `MAPPER_HOOKS` because it
is an event handler, and its updater's only effect is a write to `scrollOffsetY`, which
`RightDrawer.tsx` assigns in two places and reads in none. A later read reds that case the moment it
is added.

The `oversized` stage-all refusal (ruling 2, made testable by ruling 5) was a silent no-op, and this
is the fix as well as the case. Measured on this tree before it: `git.bulkStage` with 12,000 paths
posts one 1,033,012-byte frame, the shell's reader drops it with `{ kind: 'refused', refusal:
'oversized' }`, and the page's promise never settles — `busyAction` never cleared and
`setActionError` was never called. Both new cases red by timing out at 15s against that path.

Refused at the page's own send boundary instead, under the shell reader's own predicate rather than
a second spelling of it: `isBridgeFrameWithinCap` is extracted from `parseBridgeMessage` and used by
both sides. `sendFrame` answers `sent` / `oversized` / `port-failed`, so `sendRequest` rejects with a
`BridgeRequestOversizedError` whose message is a sentence the panel puts on its error surface, and
the members whose contract is a boolean keep it. No delivery-unknown mark: the frame never left, so
nothing ran on the desktop and the smaller retry is safe to offer.

The case runs the real chain — bridge port pair, `useMobileGitRequests`, `runGitWorkflow` — with
only react-native and the haptics seam mocked, and asserts the message that lands is a sentence and
that the busy flag is raised and then cleared.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): drop the type assertions from the two new C4.4 test files (OTA phase C, C4.4)

The changed-code quality gate named five, all in the files the previous commit added, and a fence is
not the answer to any of them. A separate commit because a reported head does not move by amend.

- The comment fixture is a real `PRComment` rather than a cast: the type's six required fields are
all this case needs, and the SAFETY disable that stood in for them was inert anyway — oxfmt had
wrapped it onto three lines, and a wrapped `oxlint-disable-next-line` matches nothing.
- The image lookup goes through `findAll` on the host tag rather than `findAllByType`, which takes a
component. Through `String`, because `node.type` is `ElementType` and React Native declares no
intrinsic elements, so the compiler reads a bare tag comparison as unreachable.
- The runners hook takes its router from `useRouteHandoff` with expo-router mocked under it, which
is how a `RouteHandoff` is obtained rather than asserted into existence. No target is pressed.
- The rejection and the diagnostic are read through narrowings instead of casts, which also drops
an `expect.any` that only type-checked because of one.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): keep a frame the page cannot serialize inside the send contract (OTA phase C, C4.4 round 1)

Round 1 finding 1. The oversized refusal moved `JSON.stringify` outside `sendFrame`'s `try`, so a
frame carrying a cycle, a `BigInt` or a throwing `toJSON` threw past the whole send path. Three
things followed, all measured here on a cyclic `params`:

- the caller was rejected with a bare `TypeError` from `JSON.stringify` instead of the
`BridgeSendFailedError` every other undelivered frame raises;
- no `send-failed` diagnostic was raised, so nothing recorded that a frame had been lost;
- `sendRequest` opens the id before it posts and abandons it on the way out, and the throw skipped
the abandon: 63 of the 64 in-flight slots were usable afterwards, against 64 on a client that sent
no such frame. Sixty-four of them and every later request is refused with nothing to say why.

`posted()` carried the same escape into the members whose contract is a boolean, where a throw is
worse still: those callers are taps and teardowns with no catch on them.

Serialization goes back inside the `try`, with the oversized refusal kept in front of the post. The
docstring said the port arm's throw is never `JSON.stringify`'s, which was exactly the assumption
that broke; it now says why the call sits where it does.

The new file is the pin: the rejection's name, the diagnostic, nothing reaching the shell, and the
slot count with a no-cyclic-frame control beside it so the count cannot pass by the cap moving. Both
changed cases red on the serialization moving back out — `expected 'TypeError' to be
'BridgeSendFailedError'` and `expected 63 to be 64`.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): pin the page's frame cap to the reader's, at the boundary and by construction (OTA phase C, C4.4 round 1)

Round 1 finding 2. Nothing held the sender's predicate to the reader's. Replacing
`isBridgeFrameWithinCap(json)` with an inline `json.length > BRIDGE_MAX_MESSAGE_BYTES + 1` passed 85
of the 86 mobile-web-shell and source-control test files on this tree, and a frame at exactly cap+1
would then be posted and silently dropped — the hang the refusal exists to end, back for every frame
in that one-unit band.

Two rules, because either alone passes against the defect:

- The boundary. A frame of exactly the cap is posted, arrives at `parseBridgeMessage` and is
accepted; a frame one byte over is refused with `BridgeRequestOversizedError`, posts nothing, and
is the same string the reader answers `oversized` to. An off-by-one reds the second.
- The census. The client reaches the cap through the shared predicate and does not name
`BRIDGE_MAX_MESSAGE_BYTES` at all, and the module that exports the predicate is the module that
parses inbound frames. A private copy that is correct on the day it is written reds here.

The overhead the boundary frames are built from is itself checked rather than trusted: a frame asked
for at exactly the cap must serialize to exactly the cap, so the constant cannot rot behind an
envelope that grew a field.

Against the mutation both new rules red — `expected null to be 'BridgeRequestOversizedError'` and
the census failing to find the predicate — while the rest of the suite stays green, which is the
finding reproduced.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): count the outbound frame in the unit both shells count it in (OTA phase C, C4.4 round 1)

Round 1 finding 3. Read off both shells rather than assumed, and they agree: iOS gates the inbound
frame on `json.utf8.count` (`MobileWebShellView.swift`, through
`MobileWebShellBridge.acceptsByteCount`) and Android on `json.toByteArray(Charsets.UTF_8).size`
(`MobileWebShellView.kt`, through `acceptsMobileWebShellBridgeByteCount`), both against `640 * 1024`.
UTF-8 bytes on each platform.

The predicate was already right. `isBridgeFrameWithinCap` decides on `utf8ByteLength`, and the
`raw.length` clause in front of it is a cheap refusal in the safe direction, not a second rule: every
code unit encodes to at least one byte, so a string over the cap in units is over it in bytes too.

The diagnostic was not. It reported `json.length` — UTF-16 code units — in a field named `bytes`, so
a frame of CJK text read as a quarter of the cap at the moment it was refused by it. It now reports
`utf8ByteLength(json)`, and the type says which unit that is.

Pinned with a 250,000-character frame of three-byte characters, which is under the cap in code units
and over it in bytes, plus a source case reading the measuring expression out of each shell. Three
mutations, all red: dropping the byte clause from the predicate reds the refusal (`expected null to
be 'BridgeRequestOversizedError'`) and the diagnostic; reporting `json.length` again reds the
diagnostic alone (`expected 250094 to be greater than 655360`), which is the defect this commit
fixes, in the number it would have printed.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(config): drop the render check's avatar assertion, which could not fail (OTA phase C, C4.4 round 1)

Round 1 finding 4. The case asserted that no avatar host was requested while both routes painted,
which reads as a proof of the web skip and is not one: no comment card renders on either page,
because the PR chain the file's own closing note names is not scripted. Reproduced here — deleting
the `Platform.OS !== 'web'` guard from `PRCommentCard` leaves the file at 5 passed.

Deleted rather than propped up. Giving the page a presence precondition means five hand-written
fixtures against five Zod schemas inside the shell double, which is exactly what the harness's
docstring says that double must not become. So the only proof of that branch is
`pr-comment-card-web-avatar.test.tsx`, which reds when the check is removed, and the render check now
says so in its header instead of implying otherwise.

What survives is a property of these two closures rather than of that component: not one request
leaves the origin while either route paints, and nothing either paints violates the policy. That one
can fail — planting a `fetch` to a provider host in a module both routes reach reds it twice, on the
console-error case and on the off-origin case, with the `connect-src 'self'` refusal in the output.

The CSP half is unchanged and was never in question: the served header is read from the Kotlin source
and the Swift twin is pinned beside it.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

2026-09-21 05:32:24 -04:00