mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 08:02:31 +00:00
debug-github-rate-limit
7
Commits
| 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
|
||
|
Jinwoo Hong
|
963da57cc3 |
test(config): preview rig readiness polls the main world, never the utility world (Chrome 152 hang) (#21963)
* test(mobile): wait for the preview frame in its main world, and probe the world that hung
Three cases spent their whole 180s on CI's Chrome inside `waitForSelector('#marker')` while
the diagnosis reported, from the same frame, `readyState: complete` and `marker: true`.
Those two readings ask in different worlds. `frame.evaluate` needs only the frame's main
execution context; a selector wait needs Playwright's injected script in Chromium's utility
world, an isolated world created per document by a command whose failure the driver swallows
and whose creation event it drops for a frame the driver considers stale. With `timeout: 0`
a world that never arrives is a wait that never ends.
So readiness is main-world polling now: the frame is resolved again from `page.frames()` on
every attempt and the predicate runs through `frame.evaluate`, still bounded by the case's
own `ctx.signal` and still ending in the diagnosis. The evaluate is abandoned after a second
so a frame that never answers cannot outlive its own replacement.
The diagnosis gains the reading that would have settled this in one run: a bounded
`utilityWorld` probe per frame, printed beside the main-world reading, so the split is
measured rather than inferred again. The competing explanation is ruled out in code --
Playwright closes a detached frame's scope with an error that every wait races, so a stale
Frame rejects rather than hangs.
Not proven red-first. Chrome 152 is the only engine that has shown this and it is not
available here; chromium 147 and WebKit 26.4 both build the utility world and both report
`utilityWorld "resolved"` for the sealed `srcdoc` frame. What is proven locally: 18 of 18 on
both engines, and a deliberately marker-less artifact still ends in the diagnosis, with
exactly one line per case naming the wait that hung.
That last part needed a fix of its own: an abort listener left behind by a wait that had
already resolved printed its stale reading at a later wait's timeout, so every timeout spoke
with more voices than it had hung waits. The listener is dropped on the way out.
In-frame `frame.click` needs the utility world too and is left alone: a main-world click is
not a user gesture, and the gesture is what those cases assert on.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): take the preview's refusal from the browser's report, not from a listener in the frame
The utility-world theory is refuted: CI's Chrome answered `utilityWorld "resolved"` on both frames
and the case failed anyway, with the widened frame reporting the artifact parsed, the CSS
background's `img-src` refusal recorded, no `script-src` refusal, and no script run. Two different
things produce exactly that reading. The policy refused the script and the frame's own listener was
not there to see it, or the sandbox refused it first, which raises no violation at all -- and a
listener inside the frame cannot tell them apart, because in the second case there is nothing for it
to hear.
So the evidence moves to where neither depends on timing: the sealed server now appends `report-uri`
to the policy it serves, carrying the arm's nonce, and the rig records what the browser reports. The
override arm's precondition is a `script-src` report from this arm's frame, waited for under
`ctx.signal` and ending in the diagnosis. Measured on both engines: a widened frame is reported for
`script-src` and a sealed one never is, while both are reported for the image the policy refuses. So
the sealed arm now waits for its own `img-src` report, which turns "no script-src refusal here" from
an unguarded absence into one measured beside a presence.
`report-uri` is additive -- it names where a report goes and changes nothing about what is enforced
-- and the first case now pins that by splitting the served header and asserting the rest is the
shipped Kotlin text exactly.
The in-frame collector stays, for the diagnosis only, and it now carries the readings that would
have answered the ordering question in one run: the init script records when it ran in each frame,
the artifact's script records the same on the document element, and the diagnosis prints both. What
the artifact wrote moved off `window` entirely for the same reason -- a page init script owns the
window of every frame it reaches. Locally the init script precedes the artifact's by one
millisecond, in every arm on both engines; the ordering on Chrome 152 is now a reading rather than a
hypothesis.
A measurement worth keeping beside the code: in a frame with no `allow-scripts` the init script runs
and its array exists, and no violation event is ever delivered to it, while the browser reports the
same refusals to the server. That is why the old `violations` assertions could not have caught this.
Red-first, all three locally: with report recording off, with the report endpoint not appended, and
with `script-src` reports alone dropped, the preconditions time out into the diagnosis and the
served-policy assertion reds too. 18 of 18 on both engines, three runs.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
* test(mobile): cover the navigation wait's sampling branch, and bind the load wait to the case
Two findings from the bots on the rig, both real.
The navigation wait's five-second sampling branch called `describePreviewFrame` after the import
that supplied it had gone. It fires only when an arm is slow, the name is evaluated before `.catch`
can attach, and `no-undef` is off, so nothing in the file or the lint run had ever executed that
line. Fixed by moving the settle waits into the readiness module, where the call sits beside the
import it needs rather than a file away from it -- the split is what let the reference dangle.
The proof is a case that drives the branch: a navigation the arm will never see, a sampling interval
passed in, and the case's own abort ending it, asserting on the reading it printed rather than on
its own absence of an error. Red-first, with only that branch's callee renamed: 2 failed, 18 passed,
`ReferenceError`. So the case covers the branch and nothing else in the file did.
The load-only arm's `frame.waitForLoadState('load')` was the one wait left that did not observe
`ctx.signal`; after an abort it kept waiting on its own timeout. It is a main-world poll on
`document.readyState` now, re-resolving the frame each attempt like every other wait here, and it
ends in the diagnosis.
20 of 20 on both engines, twice.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
|
||
|
Jinwoo Hong
|
5d13a70ea3 |
fix(mobile): keep an in-page hop local only when the session's grants cover it (OTA phase C, C2.9) (#21723)
* feat(mobile): carry what each page route declared in init (OTA phase C, C2.9) The page decides an in-page hop from `init.pageRoutes`, which says which patterns this shell would render and nothing about what each one costs. So a push kept local on the strength of the pattern alone runs the target under the opener's grants — which is how the tasks page is reached from the wide-layout sidebar without `native.clipboard.write`, and why its copy actions refuse silently. `init` now also carries `pageRouteGrants`, the manifest's own route/grant pairs, from the manifest the shell already holds. Optional in both directions: an older shell omits it and an older page ignores it, and a page that receives none keeps today's rule. No new frame kind, no cap change, no protocol bump. The grammar is the manifest's, imported rather than restated (`MobileWebBundleGrantNameSchema`, now exported for this), so a grant name the bundle could not have declared cannot reach the page through this field either. The host validates the pairs before it builds the frame and refuses the session when they fail, for the reason it already refuses a malformed route: an `init` the page would reject whole is worse than no session at all. Two files were at their line ceiling and are split rather than bumped. The pairs schema moves to `bridge-page-route-grants.ts`, which is read by both the envelope and the host, so it belonged in one place anyway. In the session reducer the three sites that each spelled out "patterns, their grants, this route's grants" become one `routeViewOf`; that is a net reduction and removes the fourth spelling before it is written. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep a hop local only when the session's grants cover it The rule the page was using is "the shell would render this pattern", and that is not the question. Grants are resolved once, from the route the shell opened, so a push kept local runs the target under the opener's list. On a wide layout the sidebar renders beside every `/h` route and pushes `/h/<id>/tasks` through this seam, so from the worktree list, agent history or the files pages the tasks page ran without `native.clipboard.write` and its copy actions refused with nothing on screen to say why. `servedHere` now means served here *and* covered: the target's declared grants must be a subset of this session's. An uncovered page route is handed to the shell exactly like a non-page route, and the shell opens it as its own session with its own grants — which is the mechanism that already exists, rather than a new one. Three answers, not two, because an absent field is not an empty one. A shell that sent no pairs keeps the old behaviour: `null` is "nobody told me", and an older shell has to keep working. A target the shell lists but names no entry for is *not* covered — the page cannot justify that hop, so it hands it over rather than guessing in the direction that loses grants. This is C3.1's explorer ⊇ preview finding without its pairwise pin: that hop is covered by this rule and stays local, and the rule scales to the sidebar, which reaches every route and which no pairwise list can keep up with. Red first on the two cases only the new rule answers; the other four are the regression guards and passed before and after. Two whole-session assertions gained `pageRouteGrants: null`, which is what the reader now returns. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): prove the sidebar hop in a browser, under the session's own grants The unit tests pin the decision; only a browser shows the control exists, is reachable at the viewport where the sidebar renders, and that the document does not move when the hop is handed over. Four cases on the shared harness, which now forwards `pageRouteGrants` (omitted when a caller names none, because an absent field is not an empty one and the page reads the difference). - Wide, session without `native.clipboard.write`: tapping Tasks posts exactly one `navigate` notify, the document stays on the worktree list, and **no new chunk is fetched** — which is what says the page did not quietly render tasks under the wrong grants. - Wide, same tap with the grant added: no notify, the document moves to `/tasks`. Without this the first case would pass on a page that simply never navigates. - Wide, shell sending no pairs at all: the old behaviour, local. An older shell must not start handing every hop over on a field nobody sent. - Narrow: asserts the absence rather than a tap. `app/h/_layout.tsx` renders the sidebar only on a wide layout, and only that header branch labels its Accounts and Tasks controls; the narrow header's are unlabelled pressables. So the hop does not exist at that viewport, and `getByLabel('Tasks')` finding nothing is the honest assertion. That unlabelled narrow header is a real accessibility gap and is not this lane's to fix. Registered in `pr.yml`'s `mobile_web_app` job beside the other render checks. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): census the in-page hops a session's grants cannot cover The rule landed in the commit before this one decides each hop; this says which hops those are, so a route's grants growing — or a new push between two page routes — shows up here rather than as a verb that silently refuses on a device. Openers are every page route, not the one that happens to push. On a wide layout `app/h/_layout.tsx` renders the worktree-list sidebar beside every `/h` route and its header pushes tasks, which is exactly why a pairwise pin is the wrong shape: the sidebar reaches everything, so the census has to be the cross product of what the manifest declares against what the source actually builds. Targets come from the hrefs the app builds, read out of `mobile/src` and `mobile/app` and reduced to route patterns, so a hop nobody writes is not pinned and a hop someone adds is. A presence case asserts the sidebar's tasks push is among them, because a census that stopped finding hops would go quietly green. Two hops are pinned as handed off today, both into tasks, which is the only route declaring more than `navigate` and `storage`. A third case asserts the other half of the rule on the manifest: a target asking for no more than its opener stays in the document. Checked that it discriminates rather than assuming: widening the worktree list's grants to cover tasks fails the pin, and restoring them passes it. **No pin was deleted.** The brief expected C3.1's pairwise explorer/preview pin to be replaced here, but C3.1 is not on this base — `MOBILE_WEB_PAGE_ROUTES` has three routes and no `files` entry, so there is nothing to remove. When C3.1 lands, its pin is this census's to subsume. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): drop an unused import from the hop census `statSync` was imported and never used; `oxlint` fails it. My error: I committed the census on a green test run without waiting for lint, the same order mistake I made earlier in this lane. Fixed forward rather than amended, because the lane forbids rewriting a commit that exists. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): fold C3.1's pairwise grant pin into the hop census C3.1 landed while this branch was open, and it brought the case this lane generalises: the explorer pushes to its own preview, that push stays in the document, so the preview runs under the explorer's grants. Its pin asserted that one pair by name. The census now covers it as a consequence rather than a rule. With the files routes in the manifest the cross product finds six more hops the session cannot cover — the sidebar into files from the worktree list and from agent history, and both files routes into tasks — and it does **not** find explorer → preview, because the preview declares no more than the explorer. That absence is the pairwise pin, derived. So the pairwise block is deleted, with its import. The rest of that file stays: its external-link seam checks and its clipboard-absence control are about what the files closure contains, which this census says nothing about. Checked the extended census still discriminates: granting the explorer `native.clipboard.write` fails the pin, restoring it passes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): prove the sidebar hop from a files route, not only the worktree list The defect is not "the worktree list pushes tasks". On a wide layout the sidebar renders beside every `/h` route, so the same hop exists from the files explorer, whose session carries `externalLink` but not `native.clipboard.write`. One opener proving the rule would have left the general case to inference, which is the inference C3.1's pairwise pin already made once. Opened on `/h/<id>/files/<wt>` with the files route's own grants, the sidebar's Tasks control posts exactly one `navigate` notify, the document stays on the files route, and no new chunk is fetched. The harness helper now takes the route and the text to wait for, so a case can open on something other than the worktree list without a second copy of it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make the render helper wait on the text its caller named The `awaitText` parameter I added in the commit before this one was never wired into the wait, so it was dead and `oxlint` failed it. The case still passed, because the files route renders the host name in its sidebar and that is what the helper was still waiting on — which is exactly the kind of accident a dead parameter hides. Third time in this lane I have committed on a green test run before lint finished. Fixed forward, not amended. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): carry route grants through the download path `onManifestRead`'s download branch set `pageRoutes` and `routeGrants` from the new manifest and dropped `pageRouteGrants`; nothing downstream recomputes it, so every first install and every OTA update reached `ready` with the default or the previous generation's pairs. The page then read each target as listed-with-no- entry and handed off every in-page hop. `routeViewOf` moves to `page-route-policy.ts`, beside the two functions it calls, to keep the reducer under its line cap without a bump; its stale neighbouring comment, which described a filter that moved into it, goes. Red first: the cold-cache and generation-change cases failed, the cached-hit case already passed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): derive census targets from navigation call sites The reachability filter was inert. Harvesting every `/h/${…}` template caught the five screens that declare their own mount pathname, two `pathname ===` comparisons and the route template types, so every declared route was reachable through its own mount: the pinned table was the all-pairs one, eight hops with the filter and eight without. Targets now come from the arguments of `router`/`navigation` `push`, `replace` and `navigate`, and of `navigateFromHostList`; mounts, comparisons and types are excluded by construction because they are not navigation arguments. Two real hops are not written as a literal, so a local binding or a call is followed one step to the function that returns the pathname: the files explorer is pushed as `{ pathname: descriptor.pathname }` and the preview as `push(createMobileFilePreviewHref(...))`. A call site whose target cannot be read is returned rather than dropped. Derived patterns go from 11 to 10; the pinned table stays at eight because all five page routes are genuinely pushed to. What changes is that the filter now discriminates: deleting the header's two tasks pushes reds the presence case and drops the four `-> tasks` rows from the pin, where the old derivation stayed green on the same deletion because `app/h/[hostId]/tasks.tsx` still declared the pathname. A push added at a real call site appears in the set. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): restore the preview-declares-something guard The pairwise pin this case replaced asserted the preview declares at least one grant before asserting the explorer covers them all; without it two empty lists satisfy the subset check and a route that lost its grants passes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): describe the route list under the handoff rule Two passages described the world before this PR: the explorer's note said the census pins its pair with the preview, and a closing paragraph left the sidebar's tasks hop open for a later PR. This is that PR. Covering the preview now buys the in-document hop rather than making it correct, an uncovered target is handed to the shell and reopened under its own grants, and the census reads the explorer to preview relation off this list rather than pinning it by name. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): mirror the manifest's tasks grants in both fixtures CodeRabbit on #21723: both fixtures declared the tasks route as `navigate`, `storage`, `native.clipboard.write` while the manifest also declares `externalLink`, so no covered-session case ever required it. Both now mirror the manifest's four, and the covered sessions hold them. That alone does not make an `externalLink`-blind rule fail, since those sessions hold every grant either way, so the unit suite gains the case that does: a session holding the clipboard but not `externalLink` must still hand the hop off. Mutating the rule to treat `externalLink` as always held reds that one case and no other. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): make a stalled hop name its own cause Both waits for the hop to land read as a bare 30 s timeout when it does not. The CI failure that sent this file back was a `TypeError` inside React Navigation that blanked the document, and it was invisible here because the error assertions run after a wait that never returns. The wait now throws with the page's own account: the pathname it stayed on, the collected page and console errors, the `navigate` notifies posted, the first 300 characters of the body, and every `.js` response since the click with its status. The response listener records every script answer rather than only the 200s, so a chunk the navigation waits on can be seen failing; the 200-only list the no-new-chunk assertions read is unchanged, as is everything the five cases assert. Kept in this file because no other render file waits on the pathname moving. Proved by mutating the rule to hand every hop off: the covered case fails naming the pathname it stayed on, an empty error list, the notify it posted and no scripts since the click — which is the handoff signature, distinct from the crash signature CI saw. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): aim the narrow hop at the control C2.10 named The narrow case asserted the absence of a labelled Tasks control, which was true only because the narrow toolbar carried no accessibility props. C2.10 gave it the wide sibling's role and label, so the assertion was red on the merge and, worse, the rule this file is about went unproven on the branch the phone actually presses. It taps that control now: at 390 px there is exactly one, and the tap posts exactly one navigate notify for the tasks route while the document stays on the worktree list and fetches no new chunk. Red first against the merged header (count 1, expected 0); with the session given native.clipboard.write the hop goes local and the case reds, which is what says the assertions discriminate. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): drop the handoff predicate's contradicted one-liner The pre-C2.9 summary said the answer is whether this document renders the target, which is exactly the claim the block comment below it replaced: the predicate now also requires the target's grants to be covered. Two doc comments on one declaration, the first of them wrong. Comment only; the 35 handoff cases are unchanged and green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert which field a route refusal blames The host builds `pageRouteGrants: <issue>` so a refusal says which of the two checked inputs failed, and nothing read it: the case counted refusals, so a host that reported the route's own verdict for a malformed pair would have stayed green while sending whoever reads the refusal to a pathname that was never the problem. The case pins the prefix, a non-empty issue behind it, and that the diagnostic and the callback carry the same string. The control is an opener that fails the other way: a malformed route reports its own issue and does not take this prefix, without which the pin would hold on any reason at all. Red first with the field branch dropped from the reason: the prefix assertion fails and the control stays green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop exporting the route filter the reducer stopped calling `implementedPageRouteEntries` and `implementedPageRoutes` were the reducer's two ways in before it moved to `routeViewOf`. The entries form had no caller anywhere afterwards and the patterns form had only this test, so the module's public surface advertised two functions no product code reaches. Both are module-local now; the surface is `matchesRoutePattern`, `pageRendersRoute`, `grantsForRoute`, `routeViewOf` and the grant list. The test reads the same list through `routeViewOf(...).pageRoutes`, which is the reducer's own view of it, so no assertion changed and no export is kept for a test. Red first: with both un-exported and the test untouched, seven cases fail with `implementedPageRoutes is not a function`; routed through the view all nineteen pass. Still discriminating, as a control: with the grant filter dropped from the entries helper, four of them fail. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): route the merged haptics cases through the policy view PR E's two haptics cases arrived with the merge calling `implementedPageRoutes`, which this branch had already made module-local, so the merged file was red with `implementedPageRoutes is not defined` on both of them. They read the same list through `pageRoutesOf`, the view the rest of the file already uses, so neither assertion changes. PR E's paragraph named that function for the filter it describes; the filter now sits in the entries helper the view is built on, so the sentence says that instead of naming a function the reader cannot see. Red: the two cases above on the merge. Green: all 21, PR E's two included. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): mirror the haptics token in every handoff fixture PR E put `haptics` on all five manifest routes, and these fixtures still carried the pre-E grant lists: tasks with four grants where the manifest now declares five. A fixture that is short the same token on both sides of the subset check agrees with the rule by accident, and would have gone on agreeing after the token stopped being universal. The pairs mirror the manifest now, and each session carries what its opener route would actually be granted, since the host narrows a route's declared grants to what the shell implements and the shell implements the token. Red first, with the token added to the pairs alone: the two covered-hop cases flip to handed-off, `stays in this document when the session already covers the target` and `keeps the hop in the document when the session covers tasks`. Green once the sessions carry it, 35 and 5. The hop census needed nothing: it reads `MOBILE_WEB_PAGE_ROUTES` itself. Measured there, all 5 routes declare the token and it is the missing grant in 0 of the 8 uncovered pairs, so it cannot decide a hop and the rule still reads only `pageRouteGrants`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(config): count C2.9's two bridge modules in the session route closure #21908 recorded this pin at 4,324 for the haptics notify module. C2.9 adds two more that the same closure reaches: the page-route-grants schema and the manifest contract whose grant grammar it imports rather than restates, both pulled in by `bridge-envelope.ts`, which the page reads to parse `init`. Named in the docstring beside #21908's sentence rather than folded into its number, because the three modules arrived from two PRs and a single count with one reason invites the next author to assume the rest. Red first against 4,324: expected 4,326. Measured on this head, not inferred -- a control worktree at pristine main gives 4,324, so the two are this branch's. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
ec82173130 |
feat(mobile): mount the terminal document in the page over its own modules (OTA phase C, C7.5) (#21809)
* test(mobile): pin the terminal WebView document byte for byte The document is already pinned as a digest, which says whether the emitted bytes moved and nothing about where. C7.1 moves the hand-written script inside it into modules the web page can import and rebuilds the document from them, and the claim that has to hold through every one of those commits is that the native screen kept the document it had. A digest cannot be the instrument for that: it fails as two hexadecimal strings. So the document is also committed as itself. The fixture is generated by `scripts/build-terminal-document-fixture.mjs`, never pasted, and the test rebuilds the comparison through that script's own substitution rather than restating it, so a fixture written by one rule and read by another cannot agree with itself. The generated xterm engine is stored as two placeholders. It is already covered by the digest test, postinstall regenerates it from whatever xterm the lockfile holds, and inlining it would put 612 KiB of vendored bytes into the file whose job is to isolate hand-written changes. Two further cases keep that from becoming a hole: the placeholders must each appear exactly once and the engine must not appear at all, and the restored document must equal the real one. Regenerating the fixture is a review event. It is only correct when the emitted document was meant to change, and the diff in that commit is the evidence. Red-first: flipping one character inside a comment in `write-queue.ts` fails both identity cases with a one-line diff naming the comment, where the digest test reports a hash. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): compare two terminal documents as programs, not as bytes The C7.1 flip commit moves the document's 57 reassigned variables onto a scope object, because a variable assigned across ES modules is a syntax error, and every read and write of them gains a qualifier. The ruling asks that the review of that commit be a test rather than a 515-line read. This is that test's instrument. It cannot be a byte comparison. Once the script's source is modules, `oxfmt` owns its style, and the repository's style has no semicolons where the hand-written document has one on nearly every line. A byte diff would therefore be dominated by changes that are not the refactor, which is the opposite of what the reviewer needs. So the comparison is over tokens: semicolons are excluded for the same reason they moved, comments never reach the stream, and one difference is allowed — `name` becoming `<qualifier>.name`, three tokens for one — which it counts and reports. It is stricter than "it still runs": a reordered statement, a changed literal, a dropped operator, a renamed local and a qualifier under the wrong object name all diverge, each reported with the token index and both sides. Acorn carries `value` on its tokens but does not declare it, so the field is read through a narrowing check rather than asserted onto the declared type. Red-first, by mutation: dropping the qualifier-name check fails the case that names it; removing the leftover-token check fails the dropped- and added-statement cases; treating semicolons as significant fails the three cases that depend on ignoring them. The acceptance case runs on the real 2,758-line script rather than on a fixture, so the instrument is known to survive everything the document actually contains. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): count each normalisation the move makes, separately Measured while extracting the first group: the document's ES5 style is not a style this repository's own rules permit. `curly` braces 279 brace-less if/else/for/while bodies, `no-unused-vars` unbinds 38 catch clauses, and 446 `var` declarators become `const`, `let` or a scope field. Those rewrites land before the qualifier is considered at all, so "the qualifier and nothing else" was never reachable once the source is a linted module. The comparison now allows exactly four classes and counts each on its own: a reference that gained the qualifier, a declaration that moved onto the scope object, a `var` that only changed keyword, a body that gained braces, and a catch clause that lost its binding. Separate counters rather than a total, because the flip commit pins each number and a total would let one class absorb another — which is the drift the pin exists to catch. The two `var` classes partition the 446, and the qualifier's 641 sites partition into references that kept their declaration and declarations that moved. Two ordering facts the cases pin. The catch rule is tried before the brace rule, or the inserted-brace rule eats the `{` that follows `catch` and the streams never resynchronise. A body braced at the very end leaves its closing brace after the baseline has run out, so trailing closes are absorbed after the walk rather than reported as a length difference. Everything outside the four classes still refuses with the token index and both sides: a changed literal, a dropped operator, a reordered pair, a renamed local, a qualifier under another object's name, a brace opened and never closed, and a brace closed where none was opened. Red-first, by mutation: disabling the catch rule, disabling the trailing-brace absorption, folding scope-field declarations into plain references, and not counting brace insertions each fail exactly the case that covers them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): make the mouse-report cell a module the page can import The first of the twelve groups the document already names. `*-injected.ts` has been splicing JS strings into the document for a while, and tests evaluate those strings, so the one-source-two-consumers shape is already there; what is missing is that a string cannot be imported by the web page, typechecked, or linted. This turns one of them into a module and adds the generator that puts it back into the document. The generator is a transform, not a bundle: a bundler orders its output by the dependency graph, and the document's order is part of what the equivalence test holds fixed. Imports are dropped rather than resolved, because inside the document every name is already in scope — that is what the single IIFE means — and `document-externals.ts` declares the names whose groups have not moved yet and emits nothing at all. esbuild prints an ESM module's exports as a trailing block, so that block is dropped whole rather than by its keyword; leaving the keyword behind would put a bare block statement in the document. Both sides of the comparison now go through that same printer before being read. Otherwise every choice the printer makes — semicolons, property shorthand, quote style — reads as a difference in the program when it is a difference in who typed it, and each would need its own rule. A script that does not parse is reported as a refusal naming its side, not thrown. `let` is contextual outside strict mode, so acorn reports it as a name and not as a keyword; without that the var-to-let rewrite the linter performs would be refused on every reassigned local. The group's counts are pinned exactly: nine references gained the qualifier (`term` seven times, `panX` and `panY` once each), nine locals became `const` or `let`, thirteen one-statement `if` bodies gained braces, no declaration moved onto the scope object and no catch clause lost a binding. The document is untouched, so the byte pin from |
||
|
Jinwoo Hong
|
bd5177801b |
feat(mobile): put the page's pickers, paste and editor fallbacks on the media verbs (OTA phase C, C7.6) (#21795)
* feat(mobile): put media picking behind a platform seam (OTA phase C, C7.6) The session screen picks images three ways — the photo library, Files, and the pasteboard — and all three are native modules a page cannot import: the codegen lookup `expo-image-picker` and `expo-document-picker` run at import throws in a browser, and the route manifest imports every route, so one of them in a page closure is the whole bundle down rather than one picker. `src/platform/media-picker.ts` is the phone's, delegating to the same three calls the screen already made. `.web.ts` is the page's: `native.media.pick`, then `read` in order to `eof`, then `release` for every handle it was handed, including the ones its caller never took — the shell holds eight staged files at a time and an abandoned pick otherwise waits out the five-minute TTL. A refusal rejects with the shell's code on it and is never folded into the empty answer that means the user cancelled. The bytes are concatenated decoded and encoded once, because the wire promises `eof` and nothing about the length: a shell answering a range shorter than the one asked for ends a chunk on a partial base64 group, and a reader joining the strings would fold that padding into the middle of the file. The census walks the session route module's own closure — the route is not registered until C7.7 — and names any module that reaches a picker or `Clipboard.getImageAsync` directly. Today that is the two modules C7.6's next commit moves, listed by name so the list goes empty rather than the rule going quiet. Inert: nothing calls the seam yet. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): put the session's paste and attach on the media seam (OTA phase C, C7.6) The terminal paste read the pasteboard through `expo-clipboard` directly and the two attach paths called `pickMobileImage`/`pickMobileImages`, so the page's closure carried `expo-image-picker` and `expo-document-picker` — native modules whose import throws in a browser. All three now go through the seam. Text is `native.clipboard.read` on the page, which the clipboard seam gains a reader for: `expo-clipboard` resolves to `navigator.clipboard` there, which needs a secure context the iOS shell's custom scheme is not. An image is `pick { source: 'clipboard' }` rather than an inline value, because a clipboard image is 24 MiB of base64 against an 8 MiB reply ceiling. The census over the session closure is empty now and asserts the seam is in it, so a rule that found nothing is one that had something to find: with the three call sites restored it names all three. `mobile-image-source-picker.ts` stays the phone's implementation, reached only through the seam's native sibling, and resolves out of the web closure entirely. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): resize a clipboard image on the page with a canvas (OTA phase C, C7.6) The paste hook carried the raster shrink inline over `expo-image-manipulator` and two `expo-file-system` writes. Both are native: the manipulator has no browser build, and the temp file exists only to work around an iOS loader that cannot decode a large base64 data URI, which a browser does not need. Split into `mobile-clipboard-image-resize.ts`, unchanged, and a `.web.ts` that decodes one `<img>` from the data URL the shell's `img-src 'self' data:` already admits, draws it into a canvas at the target size and reads the PNG back out of `toDataURL`. It reports the canvas's own size rather than the size asked for, because a browser clamps a canvas past its area limit and the downscale loop above would otherwise retry a raster that never shrank; and it awaits `decode()` rather than `onload`, which never fires for a source the browser cannot read and would leave the paste waiting on a promise nothing settles. Measured in Chromium under the shipped header, on a noise PNG because that is what PNG compresses least: 1400x1000 encodes to 5,476,032 base64 characters and converges in one pass to 368x263 and 397,220, which is 75.8% of the upload path's 512 KiB chunk. Zero policy violations and zero page errors. Red under three mutations: the source returned unchanged, a reported size the canvas did not draw, and `onload` in place of `decode()`. `computeMobileClipboardImageDownscale` moves to a leaf for the reason the upload-chunk constant has one: the check wants the arithmetic and not the upload path's RPC operations behind it. The page closure now carries none of `expo-image-picker`, `expo-document-picker`, `expo-image-manipulator` or `expo-file-system`, pinned beside the seam census. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): give the two WebView editors their plain web fallbacks (OTA phase C, C7.6) `MobileRichMarkdownEditor` and `MobileHtmlPreview` are the session closure's other two `react-native-webview` consumers. On the web that package renders the line "React Native WebView does not support this platform" where the surface was, so nothing it was mounted for works and the closure pays for a module that cannot do its job. Ruling 8: each gets the plain state it already degrades to, and no second renderer. The editor renders the Markdown source in one field on the text-input seam, so the screen around it keeps the text, every edit through `onChange`, and Save, Discard, Copy and Refresh; the degradation is the formatting toolbar, whose fifteen commands are the rich document's. The preview renders its own Source tab; the degradation is the rendered artifact, and the toggle goes with it, because a control that can only be in one position is a control that lies. Neither is smaller than a DOM renderer, which is why neither is one here. The editor's toolbar would need a `contenteditable` implementation with its own escaping, and the preview has no nested frame to sandbox agent-produced HTML in at all — the shell's policy carries `frame-src 'none'` and `child-src 'none'`. `dismissKeyboard` blurs the field rather than calling `Keyboard.dismiss`, which is a stub on React Native Web; `onKeyboardInsetChange` is never called, because it exists to correct for a WebView's covered area and on the page `keyboard-occlusion.web.ts` is the only measurement there is. The closure census names the one consumer left, `TerminalWebView.tsx`, which is C7.5's: with both siblings removed it names all three. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read a destructured clipboard alias in the media census (OTA phase C, C7.6) The census recognised `Clipboard.getImageAsync` as a property access and nothing else, so `const { getImageAsync } = Clipboard` reached the same function without ever writing one and the closure was approved. On the page that call is `navigator.clipboard`, which needs a secure context the iOS shell's custom scheme is not, so the approval was for a path that dies at the browser clipboard API. Aliases are now resolved to a fixpoint — `const pasteboard = Clipboard` makes `pasteboard` the module too, and the chain has no length limit — and a destructuring off any of them is reported at its declaration, which is the line to delete. The destructured name is read the way the import clause's is, off `propertyName` when the element renames it, so `{ getImageAsync: readImage }` is the same offence spelled differently. A binding element's `name` can be a nested pattern and a `propertyName` can be computed, so the text is taken only off a node that has one. Red-first with each shape planted in the scratch tree before the rule moved: the plain destructuring, the renamed one and the re-destructured chain were all missed. Dropping the fixpoint afterwards loses the chain; reading the local name instead of the property loses the rename. `{ getStringAsync } = Clipboard` stays unreported, because text off the pasteboard is the clipboard seam's and not this rule's. The session closure is still empty under the widened rule. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): release every item a clipboard pick answered (OTA phase C, C7.6) `readClipboardImage` destructured the first staged item and released only that one, while `pickImage` already guards the same shape through `readPicked`. `multiple: false` is what the page asks for and not what a shell promises, so a caller taking the first of several would hold the rest against the eight-handle cap until the five-minute TTL. Today's shell stages at most one on the clipboard arm, so this is the seam's own docstring made true rather than a leak in the field. Red-first with two staged clipboard items: releasing only the one read leaves `media-2` held, and the second is now returned without ever being read, which is what the single-image pick does. The refusal case is one path over both codes a pick can answer with: the registry's `native_media_handle_cap`, raised before a picker runs, and ruling 6c's `native_media_too_large`, raised once a picked item has been weighed. A code outside the seam's vocabulary floors to `native_verb_failed` rather than crossing verbatim, which is what makes naming the exact code load-bearing. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the upload chunk from its module in the resize check (OTA phase C, C7.6) The canvas resize check restated `512 * 1024` as the budget it holds a run to. A check carrying its own copy of a product constant is one that goes on passing after the upload path's chunk has moved, which is the reason the harness reads the CSP, the protocol version and the window caps out of their own sources. `readClipboardImageUploadChunkBase64Chars` joins them, evaluating the product the way the window caps reader does. Proved live by moving the constant: at 64 MiB the run reds on the fixture no longer being over the budget, and it is back to 512 KiB here. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read element access in the media census and correct two claims (OTA phase C, C7.6) Round 2, four lows. The census read `Clipboard.getImageAsync` and not `Clipboard['getImageAsync']`, which is the same call, the spelling a bundler produces, and the one a reader reaches for to get around a rule about dots. Element access with a string literal is now read the same way; a computed key is not, because its value is not in the source and guessing would report a line nobody can act on. The closure test cannot back this up — `expo-clipboard` legitimately sits in the session closure — so the scratch fixture is the whole of the evidence, and it reds with the arm removed. The fixture also could not tell the alias fixpoint from one source-order pass: every planted chain happened to be declared in the order a single walk learns it. `reverse-order-alias.ts` is declared back to front, and is valid at run time because the destructure sits inside a function the module body finishes before anything calls. Bounding the loop to one pass now reds it. The canvas resize justified reading its size back off the element by a browser clamping past its area limit. That is not what browsers do: the width attribute reflects whatever it was assigned, so the returned size is always the target. The real reason is narrower and is now what the comment and the override entry say — the dimensions and the bytes come from one element, so a caller's bookkeeping cannot describe a raster that was not encoded. The override entry also carried a stray apostrophe in `img-src 'self' data:`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): say what the clipboard contract shipped as, and read a backticked key (OTA phase C, C7.6) Round 3, three lows. The merge took main's `clipboard.ts` byte for byte, so its reader docstring still described the state C7.2 shipped: one verb on the web, and a page whose lack of an image verb degraded into the old path. The design that shipped is the other one — this seam owns the pasteboard on both platforms and the page's `readImage` runs `native.media.pick { source: 'clipboard' }` with the chunked read behind it. The prose now says that, and says that null still means an empty pasteboard while every other outcome rejects. The same merge left `clipboard` twice in the paste hook's dependency list, one from each side. Deduped. The census read a quoted element-access key and not a backticked one, so ``Clipboard[`getImageAsync`]`` escaped a rule that catches both other spellings. A template with no substitution is a string literal with a different quote, and reading only one of the two leaves the other as the way around. The computed-key plant could not see the literal-kind check at all: its variable was named `key`, so reading the identifier's text found nothing either way. It is now named after the method and holds a different one, which makes dropping the kind check a false positive on a call that reads text. Red-first: the backticked access planted before the rule moved is missed; ignoring template keys afterwards misses it again; accepting any key node reports the computed plant. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): hold canPickMedia to all three verbs and seed the census from import() (OTA phase C, C7.6) Two bot findings. `canPickMedia` answered true on `native.media.pick` and `native.media.read` alone, but every image read releases what it picked. On a route without `native.media.release` the release rejects, the cleanup swallows it by design, and the staged file stays live to the five-minute TTL: eight pastes and the next pick is refused at the handle cap, with nothing on screen to say why. A route missing one verb has no working image path, so `contents()` now says so up front rather than after four of them. Red-first: a route granted pick and read but not release answered `image: true`. The census seeded its aliases from static import and export declarations only, so `const Clipboard = await import('expo-clipboard')` produced no offender — while the bundler resolves a literal dynamic import into the closure exactly as a static one. A dynamic import is now read wherever it appears: `await` and parentheses unwrapped, the assigned identifier seeded as an alias, a destructuring off one reported at its declaration, and a picker module reported at the call, since reaching one at all is the offence. A specifier that is not a literal is left alone, for the reason a computed key is. Red-first with all three forms planted and the seeding removed: the namespace alias, the destructuring and the picker import are each missed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): seed the media census from a backticked import() too (OTA phase C, C7.6 bots) CodeRabbit: `import(`expo-image-picker`)` is as static to the bundler as the quoted form, but the census read only a string literal specifier, so a backticked one joined the closure unseen. A no-substitution template literal now seeds it the same way; the planted fixture is reported at its line and was unreported before the arm. pullfrog: the clipboard seam's docstring counted the web read as two verbs where its web sibling counts one for text and three for an image. It now counts the same way in both files. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
||
|
Jinwoo Hong
|
e6aa90ff36 |
test(mobile): certify the browser pane's golden families and render it in a page (OTA phase C, C6.5) (#21777)
* test(mobile): pin the browser pane's golden families The half pin for C6: 4 families, 15 goldens, every verdict the one C2's rule predicts. Measured per family with vitest `-t` over the full 787-golden corpus, with C1's 103 reproduced golden-for-golden as the control: 6 byte-identical, 9 result-absent-settlement. No composed `c6-page-closure.ts`: a composed table is pinned against a route and the browser is a pane, so C7's route is what composes this with C1's. The derivation census does not wait for that route. `mobileWebAppRoute- Closure` becomes one case of `mobileWebAppModuleClosure`, which takes any entries, so the pane's own closure can be read from the module. Two cases: the pane alone reaches exactly the pinned four, and the pane beside `app/h/_layout` adds exactly those four and no other, with the layout reproducing C1's 22 as the control for the difference. Closure at this base: 48 local modules alone, 34 beyond the layout, 30 under `src/browser` and four through the web siblings. The design said 23, all under `src/browser`; it was measured before C6.2 and C6.3 added those siblings, so the pin carries the re-measured number. `browser.screencast` has no golden at all, so this certifies the input path and says nothing about the frame path. Red first: with `browser.wheel` dropped from the table, both census cases fail naming the missing family; restored, the file's 10 cases pass and the parity suite reports "15 goldens in 4 families, 6 byte-identical". Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the frame budget against the shell's real frame Ruling 2's pin. `binaryEventEnvelopeBytes()` sizes the mobile view's device scale from a skeleton it builds itself, and until now its only check was another skeleton of the same shape in the same file: two copies of one assumption agreeing with each other. This measures the real thing. A frame with CDP's nine metadata fields and a real `Page.screencastFrame` timestamp, encoded by C6.1's `encodeBridgeScreencastFrame` and serialized by the real `BridgeHostSubscriptions`, posted through the host harness: 303 bytes besides the image, against a bound of 516. Held above is not enough on its own — 213 bytes of slack is room for the shell to grow the envelope by a field the page never hears about — so the bound is reconstructed exactly instead. Every byte of that slack is a number this frame prints narrower than a double can; adding those back gives 516 on the nose. The budget cases run a generated noise image at the budgeted scale, not a committed fixture: the worst case is the image JPEG compresses least, and a photograph sits a tenth of the way to it. 901,161 px at 0.545 bytes per pixel is 491,132 bytes, which the shell posts at 654,857 of the 655,360-byte cap. One envelope more and the shell drops it, which is ruling 1 read from the budget's side. Red first, two ways. Drop the metadata widening from the bound and three cases fail, the sharpest being the real shell answering the frame the page thought it could send with zero posts. Add a field to the shell's own envelope and the reconstruction fails at 516 against 548, where the existing suite stays green on all 14 — which is the drift this file exists for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): record the measured frame bytes, correcting |
||
|
Jinwoo Hong
|
b7c06900e2 |
fix(mobile): give reanimated mapper hooks the inputs esbuild never writes (OTA phase C, C1.10) (#21592)
* refactor(mobile-web): extract the page render harness The shell double, the CSP/bridge constant readers and the bundle server were private to mobile-web-app-render.test.mjs, so a second check against the same page had no way to reach them. Moved as-is into a module both can import; the double also gained a `replies` map so a check can answer one method and leave the refusal in place for everything else. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): give reanimated mapper hooks a dependency array The bottom drawer never slid onto the screen in the web shell page: `progress` animated to 1 and `withTiming` reported finished, but the sheet kept the translateY of the animation's first frame and sat one viewport below the fold, with its invisible backdrop swallowing the next touch. Cause, bisected in the browser: `useAnimatedStyle` reads its mapper inputs from `updater.__closure` (hook/useAnimatedStyle.js), which only Reanimated's Babel plugin writes. The page is bundled by esbuild, which runs no Babel, so `__closure` is undefined; with no dependency array either, `inputs` is empty and `startMapper` registers a mapper that listens to no shared value. It runs once and never again. Reanimated does throw for exactly this, but behind `__DEV__`, which the bundle builds out, so the page reports nothing. The rAF loop stopping after one write is the observable end of it. Not a WebKit fault. Headless Chromium parks the sheet the same way (translateY(843) vs WebKit's translateY(841)), so the earlier JavaScriptCore-vs-V8 reading does not hold, and the pin added here runs on both engines rather than on Chromium alone. WebKit is downloaded in the mobile_web_app job for it. Every mapper-backed call site takes the same array, not just the drawer's: RightDrawer and DragReorderList are the same defect on the same bundler. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): census the reanimated hooks that need a dependency array The drawer pin covers MountedBottomDrawer only, and the failure mode is silent: a new `useAnimatedStyle`, `useAnimatedProps` or `useDerivedValue` without an array animates once on the phone's native build and freezes in the web page, with no error on either. Parsed rather than grepped so a call spanning lines, or one whose second argument is not an array, is still seen. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web): state motion-on as the drawer pin's precondition Under `prefers-reduced-motion: reduce` Reanimated finishes `withTiming` in one frame, so a mapper that only ever runs once still writes the final translateY and the pin goes green on the broken build. Measured: the unfixed bundle under reduced motion lands at translateY(0) with the sheet on screen in both engines, which is also what the Android emulator does with animator scale off — the same single write, not a healthy animation. The context now says no-preference and the page is asked to confirm it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): census useAnimatedReaction, whose deps are its third argument Same fallback as the other three (hook/useAnimatedReaction.js:26-34), so the same silent freeze applies. Its shape is not the same: the array is argument three, behind `prepare` and `react`, and both callbacks run inside the one mapper it starts, so both count as updaters. Indexing it like the others would have read the `react` callback as the array. No call site today; this is the gate for the first one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): require the dependency array to list every value the updater reads An array proves a call was written, not that the mapper listens to everything it reads. On web `inputs` becomes exactly that array (hook/useAnimatedStyle.js:338-341), so a value read but not listed is a value the mapper never hears about: the updater stops re-running when only that one changes. Same freeze as no array at all, in one prop rather than all of them. Reads only. The first fixture caught this check counting `opacity.value = v` as a read, which it is not -- a written value is an output, and demanding it in the array would be noise at every `useAnimatedReaction`. Assignment targets and increments are excluded; a value both read and written is still required. Verified against the tree by dropping `translateY` from the bottom drawer's array, which the census names at mounted-bottom-drawer.tsx:286. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): resolve the hook through the file's imports, not by spelling Matching the callee's text both missed and invented. `useAnimatedStyle as useAS` and `Reanimated.useAnimatedStyle` are the same hook wearing another name and went unchecked; a local helper that happens to be called `useDerivedValue` is not this hook and would have been flagged. Each local name is now resolved through the file's imports from `react-native-reanimated`, named, aliased or namespace member. A second argument that is not a literal array now counts as present rather than missing: the hook only needs an array to exist, and this file cannot see what a hoisted `const deps = [...]` holds, so completeness covers literal arrays only. Resolution can fail closed, which would read exactly like a clean tree, so the census now asserts it saw the calls before asserting none are missing. Checked against the tree by dropping `translateX` from RightDrawer's array, which it names at RightDrawer.tsx:156. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web): select the drawer sheet by name, not by its corner radius The pin walked up from the handle to the first ancestor with a 16px top radius, so it found the sheet through a styling token. Change that radius and the pin reports `sheet: false` -- a red naming the selector rather than the animation it exists to watch, on a change that broke nothing. The sheet now says what it is. `testID` on the RN side renders as `data-testid` on web (react-native-web createDOMProps/index.js:832), which is the one line of product change this needs. Re-verified after retargeting: still red on both engines with the dependency arrays removed (translateY 843.271 chromium, 841.447 webkit), green with them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile-web): say why the motion option must precede navigation Reviewer follow-up on the reduced-motion guard. The context option and the `goto` order are both load-bearing, and nothing in the file said so: Reanimated reads `matchMedia('(prefers-reduced-motion: reduce)')` once into a module-level const at import (ReducedMotion.js:8-10), so a `page.emulateMedia()` after navigation would leave the assertion passing over a value already latched true. Comment only. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web): name the drawer pin's precondition instead of asserting past it CI's Linux WebKit failed this pin at `matrix(1, 0, 0, 1, 0, 844)` -- exactly the viewport, the mount-time value, not a first-frame 843.x. Nothing animated there, so the pin was reporting a parked sheet without being able to say whether the mapper was subscribed. Two different faults, one message. `requestAnimationFrame` separates them and sheet writes do not. `withTiming` schedules a frame per step (valueSetter.js) whether or not a mapper listens, so frames across the window mean the shared value moved; the assertion now names that. Counting sheet writes as the precondition inverts the diagnosis: measured on the broken build, "written more than once" fires first and calls the defect this pin exists to catch an engine that does not animate. Sheet writes stay, as a second statement of the subject and as context in the transform failure, which now reads "1 style write(s) on the sheet across 30 frame(s)" on the broken build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web): wait for the drawer to arrive, not for a clock The pin paused a fixed 1s after the sheet opened and then read the transform, which makes it a race on a loaded runner: a healthy engine that is merely slow reads as parked, and the red names the transform rather than the wait. It now waits for the settled transform, times out at 15s, and asserts on whatever it found either way, so a genuinely parked sheet gives the same red with the timing assumption removed. On the broken build that red now reads "1 style write(s) on the sheet across 3635 frame(s)", which says the fault in one line. Aimed at CI's Linux WebKit red rather than proven against it: eight container runs on the Playwright Linux image never reproduced that failure. See the report for what the container did and did not show. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web): stop asserting on the sheet's style-write count The count cannot carry an assertion in either direction. Measured under `--cpus=0.35` in Playwright's Linux image, a healthy page starved of frames reaches translateY(0) in a single write, because `withTiming` covers the whole 180ms in one step when one step is all the frames it gets. "Written more than once" would have redded that page, which is a CI runner under load -- the exact situation this pin keeps meeting. So the transform is the only subject, `requestAnimationFrame` during the window is the only precondition, and the write count is context in the failure text. Also worth recording against the CI log: exactly `matrix(1, 0, 0, 1, 0, 844)` is reproducible here on the broken build, as the single mapper run landing at progress 0. It is the mapper's signature as much as a dead engine's, so it does not on its own say which failed -- the frame and write counts now printed beside it are what separate them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): count a value read under a unary operator as a read `isWriteTarget` took any prefix-unary parent for a write, so `!hidden.value`, `-offset.value`, `+x.value` and `~x.value` were dropped from the reads the dependency array has to list. A style that gates on `!hidden.value` would have passed the census while its mapper never listened to `hidden` -- the exact freeze this file exists to catch, hidden by the check meant to catch it. Only `++` and `--` mutate, so the prefix branch is narrowed to those two. Postfix needs no narrowing: `++` and `--` are the whole set there. Red-first with a negation fixture and a unary-minus fixture; the increment fixture holds the other side, that a value only incremented is still not required. Found by a review bot on #21592. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb |
