166 Commits

Include renames

Author	SHA1	Message	Date
Jinwoo-H	4ea945bc7e	feat(mobile): let the text-input census read a literal already clear of the floor (OTA phase C, C7.2, ruling 12) ... The floor is the rule and the seam is the mechanism. A binding rule alone made the custom-key capture field an offender at 22, where nothing can zoom, and the only way to satisfy it was to lower a one-character field to 16 — the tail wagging the dog. The seam's web half now exports the floor it already computed `Math.max` against, and the census reads that number out of that file rather than carrying a second copy of 16. The rule becomes "the seam's binding, or a literal at or above the floor", with no per-site exemption: a literal under the floor is still reported, which is the case the seam exists for. A tree whose seam declares no floor is refused rather than judged against a number the census invented. So the capture field goes back to 22 on both platforms and its split, its override entry and its parity test go with it. The chat's two fields stay split, because 15 is under the floor however it is spelled. Red-first: with the rule removed, a planted literal 16 and a literal 22 are both reported and the refusal case does not throw; a literal 15 is reported either way. All three route closures that run this census — session, source-control, review — report 0 offenders and 0 unresolved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-20 07:02:47 -04:00
Jinwoo-H	20a0ead745	feat(mobile): open the session screen's three external URLs through the platform seam (OTA phase C, C7.2) ... The landed external-link census, run over the session route's closure, reports three modules reaching react-native's `Linking`: a terminal link tap whose open mode is the phone's browser, and the two WebView-backed readers, each of which sends a tapped link to the system browser rather than navigating the artifact away. Inside the shell `Linking.openURL` calls `window.open`, which both shells refuse and which resolves either way, so all three reported success into a tap that did nothing. The seam also stops swallowing the failure: each site caught and discarded, and `openExternalLink` names it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-20 06:03:54 -04:00
Jinwoo-H	cad2bb128c	feat(mobile): put the session screen's nine text inputs on the web font seam (OTA phase C, C7.2) ... The landed text-input census, run over `app/h/[hostId]/session/[worktreeId].tsx`, reports nine sizes that do not come from `TEXT_INPUT_FONT_SIZE`. Six declare the app's body size and move in place, which is the same number natively. Three do not — a 22px key-capture field and the chat's two 15px fields — so each gets a `.web.ts` sibling of the address bar's shape, with a shared base so the two halves can differ in nothing but the size. The capture field is the one the move shrinks rather than raises: 22 already clears the focus-zoom floor, and the census reads the seam as a binding rather than as a number, so there is no expression that keeps 22 and still says where the size came from. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-20 05:56:04 -04:00
Jinwoo HongandClaude	ee61e3bd41	fix(mobile): measure the keyboard from visualViewport inside the page (OTA phase C, C4.2) (#21735) ... * feat(mobile): measure the keyboard from visualViewport inside the page (OTA phase C, C4.2) react-native-web's `Keyboard` is a stub: `addListener` returns a subscription that never fires and `isVisible()` is always false. A screen inside the shell's page that waits for `keyboardDidShow` waits for the life of the document, and the software keyboard covers whatever sits at the bottom of it. Two C4 screens are text entry at the bottom. `platform/keyboard-occlusion` is the pair. The native file carries the source-control hook's logic unchanged, events and clamp and the comment that travels with it. The web sibling reads `visualViewport`: the layout viewport keeps its size and the visual one shrinks, so the occluded strip is `innerHeight - (height + offsetTop)`. `offsetTop` is in it because a scrolled or pinched visual viewport sits partway down the layout viewport and the strip below it is not keyboard; dropping the term reds two cases. It listens on `resize` and `scroll` — the browser scrolling a focused input into view moves the offset without resizing anything — and reads once at mount, because a composer opened over an already-raised keyboard receives no event at all; dropping that read reds a third case. `useKeyboardAvoidingPadding` is a second name rather than a `Platform.OS` branch at the call site. Natively it is 0 and subscribes to nothing, so a composer that asks for it renders exactly as often as it does today; `KeyboardAvoidingView` has already moved it and padding would move it twice. On the web it is the whole of the avoidance, that view being driven by the events this file exists because the page never receives. No `visualViewport` answers 0 rather than guessing. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): lift the commit bar and the note composer inside the page (OTA phase C, C4.2) The two consumers move onto the seam. The hub's hook becomes one line and keeps its name, which is what the hub's state calls the number. The note composer takes the padding as a style on the `KeyboardAvoidingView` it already had: natively that is 0, so the prop is `undefined` and the phone renders exactly what it rendered before; inside the page it is the strip the keyboard covers, which is the only thing that moves the composer there. The census is over both future route closures rather than over the two call sites: `platform/keyboard-occlusion` is the one module in either closure allowed to name the stub. Red first at the base commit — run in a throwaway worktree at `9309350864` rather than by setting the fix aside — it named `use-mobile-source-control-keyboard-lift.ts` as a subscriber outside the seam and found the seam's web file in neither closure. `mounted-bottom-drawer.tsx` is exempt by name, and the census asserts the exemption is really in both closures so it cannot outlive its subject. It reads more than a height — `Keyboard.metrics()` for a sheet opened over a raised keyboard, and each event's `duration` to animate with it — which the seam does not model, and it sits in C1's, C2's, C3's and C5's closures too, so moving it is a change to every page rather than to this domain. Its listeners are inert on the web the same way, which is why the composer inside it takes its own padding rather than inheriting one. No render-check case: measured, none of the five registered routes reaches the seam, the commit bar or the composer, and a headless browser cannot shrink the visual viewport independently of the layout one anyway. C4.4 carries it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): type the keyboard harness instead of asserting its fields (OTA phase C, C4.2) The changed-code gate flagged the two `as` casts in the hoisted harness. A return type on the `vi.hoisted` callback says the same thing and is checked rather than asserted, which is the shape the host-list route test already uses. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read a pinch zoom as no keyboard, and test the clamp (OTA phase C, C4.2 round 1) Round-1 folds plus CodeRabbit's exemption point. **A pinch zoom read as a keyboard.** A 2x zoom shrinks the visual viewport by exactly as much as a half-screen keyboard, so the commit bar and the composer moved on a page nobody was typing into. A `scale` other than 1 answers 0. Geometry alone cannot tell the two apart and a stored "no keyboard" baseline would be a heuristic, so a keyboard raised while zoomed is the accepted rare case rather than a guess. `scale` is read defensively because older WebViews do not implement it, and taking its absence for zoomed would answer 0 for every keyboard on them; mutating the guard to key on absence reds both cases. **The clamp had no test.** A bare subtraction left all nine cases green. The case is a visual viewport taller than the layout one, which mobile Safari reports mid-scroll and which would have pushed the commit bar down the screen instead of up. **One guard, where the test reaches it.** `occlusion`'s `viewport === undefined` arm was unreachable: the effect returns before calling it, and the absence case exercised that one. Deleted, and the remaining case says which guard it proves. **The census exempts two files, not a directory.** `startsWith('src/platform/')` would wave through a later `src/platform/*.web.ts` that subscribed to the stub directly, which is the defect this census exists for. Named exactly, with a planted subscriber beside the seam as the fixture; restoring the directory filter reds it. **And the moved comment claimed an inset it never subtracted.** Deleted. Correcting a comment that was false where it came from is not a rewrite of the logic the move carried: no statement moved with it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the page at scale 1 so the zoom guard is not the keyboard path (OTA phase C, C4.2 round 2) Round 2's finding changes what the zoom guard costs. iOS auto-zooms on focus of any input under 16px; both consumers' inputs are 14px (`typography.bodySize`), and the page's viewport meta set no `maximum-scale`. So `scale !== 1` was not the rare pinch the guard was written for, it was every focus — and the seam would have answered 0 on the one flow it exists for. The guard stays and the premise is fixed instead: `maximum-scale=1` in both places the page's meta is written, the built document in `build-mobile-web-app-bundle.mjs` and the bootstrap `index.html`. iOS honours it for the focus auto-zoom and has ignored `user-scalable=no` since 10, so a deliberate pinch still works; the input sizes are untouched. C4.6 step i is what settles it on a device. Three test changes and one correction. The census took a `rootDir`, as `findWebSiblings` does: it planted `src/platform/other.web.ts` in the real tree while the overrides census walks `mobile/src` in a parallel worker and would read it as an unlisted override. It plants under `mkdtemp` now, and writes the two seam files there too, so the empty result for them is the name exemption working rather than those files happening not to subscribe. A case for the ruling itself: scale 2 with a viewport shrunk past what the zoom explains answers 0. Dropping the guard reds it and the pinch case together. `useKeyboardAvoidingPadding` is rendered through the test renderer now instead of called outside one, with a counter on `Keyboard.addListener`. Making the native hook return `useKeyboardOcclusion()` reds it at two calls; the old shape could not see that, because a hook read outside a component never runs its effects. Item 4 did not hold as written. `window.visualViewport ?? undefined` is not a no-op: the DOM declares the property `VisualViewport | null` and an older WebView omits it entirely, so the coalesce was normalising both shapes into one `=== undefined` check. Removing it and testing only for `null` throws on the absent-viewport case (reproduced: `Cannot read properties of undefined (reading 'scale')`). The coalesce is gone and the guard names both shapes instead. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): raise the two page inputs to 16px on web instead of pinning the page scale (OTA phase C, C4.2 round 2) `maximum-scale=1` is reverted from both metas. It fixed the right problem in the wrong place: Android WebView honours it and iOS ignores it for pinch, so the cost of stopping an iOS focus auto-zoom was deliberate zoom on Android, taken from the users who need it most. The font size is where it belongs. `src/platform/text-input-font-size.ts` is the app's body size and `.web.ts` is that raised to 16, the size below which iOS zooms on focus and does not zoom back. The commit bar and the review note composer take their `fontSize` from it. A phone renders what it rendered before: the native constant is `typography.bodySize`, so both style objects are unchanged there. `Math.max` rather than the literal, so a theme that raises the body size past 16 keeps its own value. The zoom guard stays and its rationale is rewritten to say what now keeps the ordinary path off it: the inputs clear the floor, so a scale other than 1 means a user pinched rather than an input took focus. The pin is a unit case because the render check has no route to open yet. Three assertions and what reds each: the web constant below 16 reds the first, and a style going back to `typography.bodySize` reds the third, which reads the two stylesheets as source because a node test resolves the native sibling and would otherwise pass while shipping 14px to the web. The overrides census covers the swap itself. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): put every text input in the two closures on the size seam (OTA phase C, C4.2 round 2) The 16px floor reached two inputs and the rationale claimed a page. Eight more text inputs in the same two closures still declared 14px, so a focus on any of them zoomed the document and the occlusion seam — which reads a scale other than 1 as no keyboard — stopped lifting for the rest of that session. "A scale other than 1 means a pinch" was false while they were there. All eight go through `TEXT_INPUT_FONT_SIZE`, named by the census before the change: src/components/MobileSearchField.tsx:175 src/components/SmartWorkspaceAdvancedFields.tsx:84 src/components/SmartWorkspaceSourceField.tsx:137 src/components/new-worktree-form-styles.ts:125 src/components/pr-sidebar/MobileLinkPrForm.tsx:120 src/components/pr-sidebar/mobile-pr-sidebar-styles.ts:299 src/components/pr-sidebar/pr-comment-composer-styles.ts:20 src/components/smart-workspace-source-drawer-styles.ts:60 Every one declared `typography.bodySize`, so there was no input carrying a size of its own to preserve and the phone is byte-identical again. Each of those style keys was checked for consumers first: all of them are read by a `TextInput` and nothing else, so raising the web value moves no other element. The census is the rule rather than the list. Over both closures it resolves each `TextInput`'s style to the module that really declares the size — following a spread, because both seam-served inputs are reached through `{ ...base, ...list }` and a walk that stopped at the first module would have called their offence absent — and names anything not on the seam as `path:line`. A style with no `fontSize` inherits and is not an offender. Presence precondition: the seam's web file is in the closure, so an empty list cannot mean a page with no inputs. Run against the previous head it prints exactly those eight for both routes; three fixtures under mkdtemp cover the cross-module line, the spread, and the two non-offender shapes. The web test's rationale named `maximum-scale=1`, which is gone; it names the input floor now. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): make the input census prove its own enumeration (OTA phase C, C4.2 round 2 addendum) The offender list only says every text input is on the seam if every text input was read, and the walk could not tell "this key sets no size" from "I could not follow this style" — both answered nothing, so a resolution failure would have read as a clean input and the rule would have gone quietly vacuous. `resolveStyleKey` answers three ways now: not found, found with no size, found with one. `unresolvedTextInputStyles` reports the first as `path:line (key)`, and the census asserts it is empty for both closures beside asserting the offender list is. Measured rather than assumed, which is what the addendum asks for. The two closures hold 12 `TextInput` elements and 13 style references; none uses an inline style object and none is without a style prop. All 13 resolve, 12 to `TEXT_INPUT_FONT_SIZE` and one — `styles.disabled`, combined with `styles.input` on the same input — to a style that really sets no size. The reviewer picker is in that list at `mobile-pr-sidebar-styles.ts:300`; it was already on the seam from the previous commit, which enumerated from the closure rather than from the review. A fourth fixture plants both shapes side by side: a style with no size, which is not an offender, and a style reached through a package import, which is named. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): close three holes in the text-input census (OTA phase C, C4.2 fold 3) All three of CodeRabbit's findings are on the completeness property the addendum bought, and all three reproduced before the change: each shape below answered 0 offenders and 0 unresolved, which is to say it vanished. Inline style literals. The walk recorded only `object.key` references, so `style={{ fontSize: 14 }}` was neither an offender nor a hole. Style props are flattened structurally now — arrays, spreads, `?:`, `&&` and parentheses down to the expressions that can really land — rather than walked as a subtree, which had the second bug of descending into an inline literal's own properties. `&&` is followed because `[styles.input, disabled && styles.disabled]` is the shape this tree actually uses; `null`, `undefined` and `false` branches contribute no style and are dropped rather than called unfollowable. An inline literal resolves in place, and any other shape — a call, a bare identifier — lands in the unresolved list. Source-order precedence. `{ input: safe, ...legacy }` is `legacy.input` at runtime, and answering direct keys before spreads read `safe` and called the override clean. Properties are walked in reverse source order now, direct keys and spreads in one pass, first answer wins. The seam by binding. `size.text !== SEAM_EXPORT` accepted anything spelled `TEXT_INPUT_FONT_SIZE`, so a local `const TEXT_INPUT_FONT_SIZE = 14` two lines up passed, and so did an import of that name from any other module — the regression the seam exists to stop, wearing its name. The identifier is resolved in the declaring module and accepted only as an import from `src/platform/text-input-font-size`. That last one changes what a fixture must say: the existing seam case spelled the name without importing it, so it plants the seam module and imports from it now. Six new fixtures, all six red on the previous walk. Re-measured at this head, both closures: 12 `TextInput` elements, 13 style references, 12 on the seam, 1 sizeless (`styles.disabled`, combined with `styles.input` on one element), 0 offenders, 0 unresolved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-09-20 01:29:30 -04:00
Jinwoo HongandClaude	f3bda1bf3e	refactor(mobile): seam moves and the shared shell route guard for the source-control domain (OTA phase C, C4.1) (#21732) ... * refactor(mobile): open PR sidebar URLs through the external-link seam (OTA phase C, C4.1) The three openers in the PR sidebar called `Linking.openURL` directly: a check's "open on the web", a comment's permalink, and a link inside comment Markdown. Inside the shell's WebView react-native-web routes that to `window.open(url, '_blank', 'noopener')`, which both shells refuse and which resolves anyway, so the tap reports success and opens nothing. Both C4 routes reach the sidebar, so both would have shipped that. The census is the point rather than the three edits. It derives the two future route closures through `mobileWebAppRouteClosure` and holds every module in them to the seam, so a module entering either closure later is ruled without anyone adding it here. Red first it named all three by `path:line`: CommentMarkdown.tsx:2, PRChecksSection.tsx:2, PRCommentCard.tsx:2, on both routes. The walk it runs was the third copy of one function, so it moves into the seam's own module beside the predicate that module exists to share, and the files and tasks censuses now call it too. It reports `path:line` where the copies reported paths; `reachesReactNativeLinking` keeps its name and its meaning and is now derived from the line list, so there is one rule. An empty offender list is empty in either shape, which is why repointing the two landed censuses moves nothing they assert. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): copy through the platform clipboard seam in review and conflicts (OTA phase C, C4.1) The two copy actions both C4 routes reach called `expo-clipboard` directly: the conflict section's refresh commands and the review sheet's notes. On the web that module is `navigator.clipboard`, which needs a secure context — the iOS shell serves the page from a custom scheme and Android from https, so the path works on one platform and silently not on the other. `useClipboardWriter` is the seam C2.4 landed for exactly that. Red first, the census named both routes: `ExpoClipboard.web.js` in each closure, and `src/platform/clipboard.web.ts` in neither. Both call sites also stopped ignoring whether the pasteboard took the text. The conflict section already returned on a throw, so the seam's rejection reaches an arm it had. `copyNotes` had none and its only caller is `void controller.copyNotes()`, so a rejection would have been unhandled with "Review notes copied" left on screen; it now catches and reports through the screen's own error line. That is the one behaviour change here and the reason `clipboard` joins its dependency array. Its suite mocked `setStringAsync` as resolving `undefined`, which the seam reads as a pasteboard that refused, so every copy would have gone down the new refusal arm unseen. The mock now resolves `true` and two cases pin both arms; mutating the catch away kills the refusal one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): take the source-control router from the handoff seam (OTA phase C, C4.1) The hub takes its router once, in the openers hook, and passes it down to the runners and the panel — so one `useRouter()` is this domain's whole reach into routing, and it was expo-router's own. Inside the shell's page that posts no `navigate`, so the hub's push to review would stay in the document whatever its grants, and its push to a native route would paint Unmatched over the page. Inert today: no C4 route is registered yet. `use-mobile-source-control-runners.ts` is the second case and the reason the rule reads value imports rather than identifiers: it named expo-router only to write `ReturnType<typeof useRouter>`, a value import in a type position that keeps the module in the graph. `RouteHandoff` is the seam's own name for that type. The census is C3.1's, and its walk moves to `src/navigation` rather than being copied a second time; each domain keeps only its own evidence, the list of modules meant to hold a router. Red first it named both modules on the expo-router rule and reported no handoff caller at all. The C2.9 hop census is unchanged and cannot move: its targets come from the call sites, and the derivation over this tree returns the same ten targets and the same 26 unresolved sites before and after this commit, byte for byte. Its `HANDED_OFF` pin is over registered routes, of which this adds none. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): move the shell route guard out of the files domain (OTA phase C, C4.1) `src/files/mobile-file-shell-route.ts` was never about files: it parses a route against `BridgeInitRouteSchema` and builds the key a shell screen remounts on. It moves to `src/mobile-web-shell/shell-screen-route.ts` as `shellScreenRoute` / `shellScreenRouteKey`, with its test. The move is pure — with the rename applied and comments stripped, the old file and the new one diff to nothing. Three routes had grown their own copy of the call and two had none. The copies go: `agent-history` and `tasks` now ask the shared predicate, which is the same schema and the same fallback they already had. `index.tsx` had no guard at all, so a `.` or `..` host id was handed over and came back as "Update Orca to open this workspace" painted over the native list behind the switch; it now stays native. That is the one behaviour change here, pinned red first and killed by mutation. `web.tsx` keeps handing that route over on purpose and is exempt by name: its fallback is a redirect to the route the user came from, so the host's own verdict is the better answer there, which `mobile-web-shell-route.test.tsx` already pins. No `key=` expression moved; the three switches still key differently (host id, pathname, pathname plus params) and making them agree is a behaviour change for another PR. The census walks the route tree rather than a list, so a switch added later is held to both rules without being added here. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): mount the copy cases without a client instead of casting one (OTA phase C, C4.1) The changed-code gate flagged four type assertions on the two cases added with the clipboard seam: they stubbed an `RpcClient` the way the file's older cases do, and the gate reads changed lines. Copying reaches no client at all, so they mount without one, which is both cast-free and a truer statement of what the path needs. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): say what the censuses report and sort the red list by line (OTA phase C, C4.1 round 1) Round-1 folds, four wordings and one ordering. `externalLinkOffenders` said "every call site" and reports the line the name enters the module: a named import once, however many times the module calls `openURL`, because the import is what the rule is about and what has to go. Only a namespace import reports its uses, there being no single line to name. The docstring now says that. Its red list sorted the rendered strings, which puts `:10` before `:2`. It now sorts by path and then by line as a number. Pinned against a written fixture rather than the tree, because the case needs a module with sites either side of line ten and no module in a closure has to keep having one — the first fixture used lines 11 and 12, where both orders agree, and the mutation walked straight through it. `shell-screen-route.test.ts` still named the files screens in its describe after the guard stopped being theirs; it names what a switch does now. `router-seam-census.test-support.ts` excluded `.test-support.ts` from the walk, which the files census it was extracted from never did. Dropped, so both censuses walk the same set. Inert today: neither `src/files` nor `src/source-control` holds such a file, so it only decides the next one. The `web.tsx` exemption claimed a redirect "that looks like nothing happened". What was measured: adopting the guard there sends a `..` deep link through `Redirect href="/h/.."` to the host route, which this PR keeps native, so the developer lands on the host list with nothing said about why the page did not open. The route is `__DEV__`-only and the host's own failure screen is the better verdict. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): see every react-native alias, normalise the host id, surface a refused copy (OTA phase C, C4.1 CodeRabbit) Three bots findings on #21732, all real. **Every alias, not the first.** `reactNativeLinkingSites` found namespace imports with `exec` and inspected only the first binding, so a module importing the namespace twice and calling `Linking.openURL` on the second reported no site at all. It reads every alias now and counts a line once however many meet on it. Red first with exactly that fixture. **The host id can be an array.** `app/h/[hostId]/index.tsx` read it bare, and Expo Router answers a repeated key with one: `String(['a','b'])` is `a,b`, `encodeURIComponent` makes that the single segment `a%2Cb`, and the segment rule accepts it — so the shell opened a page for a host nobody has. Through `firstParam`, as the other four switches do. Red first it handed over `/h/host-1%2Chost-2`, and the empty-array case found a second one: `[]` is truthy, so a bare read built `/h/` and handed that over too; `firstParam` answers `''` and the route stays native. That import pulls the source-control screen state, and with it the lucide barrel whose `LucideProvider` re-export is the gap the web build patches, so the suite mocks the barrel as the other suites do. It moves no page closure: the closure resolves `index.web.tsx`, which this does not touch, and the index route still measures 3426 modules, 289 local, 22 families. **A refused copy said nothing.** `PRConflictingFilesSection` caught the rejection and returned: no tick, no message, a tap indistinguishable from one that copied. The label now carries the third state, reusing the tasks page's own wording for it, and the component has its first test. Mutating the failure arm away reds it. Its prop narrows to `Pick<PRInfo, 'mergeable' | 'conflictSummary'>`, which is what it reads and what let the test drop a cast the gate flagged; every caller holds a full `PRInfo` and satisfies it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the censuses' subjects as code, not as text (OTA phase C, C4.1 round 2) Round-2 additions. A separate commit because `d4b14d54e5` was already made and this lane does not amend. **The seam walk parses now.** Matching `X.Linking` in the text named it inside a comment that talks about it and inside a string that quotes it, and the named-import regex did the same for a commented-out import. Checked against the previous implementation, all three fixtures were red there: the comment case reported lines 2 and 3, the string case reported the string's line beside the real call, and `// import { Linking } from 'react-native'` reported line 1. The walk builds a `SourceFile` and reads import declarations and property accesses, so comments and strings are gone by construction and the quote styles stop being a special case. Cost measured on the three closure censuses: 3.3 s, unchanged. **The route census reads the call, not the import.** A switch that keeps the import while the call goes — deleted, or moved behind a branch that never runs — looked exactly like one that asks. It now needs both, proved by mutation: dropping `shellScreenRoute(` from `tasks.tsx` while leaving its import names `tasks.tsx`. A fixture carries the same rule in isolation, since every switch in the tree calls what it imports and the case would otherwise be unfalsifiable against it. **And recognises a switch by its import** of `MobileWebShellScreen` rather than by `<MobileWebShellScreen` in the text, so an alias or a line break the formatter chose cannot hide one and a comment cannot invent one. The `app/h/[hostId]` root stays written out: deriving it from the manifest is not a one-liner from here, the manifest being an `.mjs` this test reads as text. What ties the two together instead is a new case asserting every registered pathname starts with that prefix, so a page route outside it fails rather than going unwalked. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the imported name, not the local one (OTA phase C, C4.1 CodeRabbit) `import { Linking as NativeLinking } from 'react-native'` went through the census untouched: the walk compared the specifier's local binding, which is `NativeLinking`, while the imported name lives in `propertyName` when a specifier renames it and only in `name` when it does not. Reproduced before the fix — the aliased import with a call beside it reported no site at all. Reading `propertyName ?? name` closes it in both directions. A module that renames `Linking` is named at its import line like any other, and a module that imports `View as Linking` is no longer named for a local binding that reaches nothing. The second was a false positive the old comparison had by construction. One more of the same class, found while checking and verified rather than assumed: `import RN from 'react-native'` typechecks in this project (tsc accepts it), and a default binding is the whole namespace exactly as `* as RN` is, so `RN.Linking.openURL` through it was invisible too. The default binding joins the alias set, which already reports uses rather than the import. Four fixtures. Three red on the previous walk: the renamed import, the local-only `Linking`, and the default import. The fourth — an alias imported that never reaches `Linking` — passed before and is here to hold the other half of the rule, that importing react-native is not itself the offence. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): parse each module as its own kind, and read re-exports (OTA phase C, C4.1 CodeRabbit) Two ways a module reached `Linking` past the census, both reproduced before the change. Every file was parsed as TSX. In a `.ts` module `const id = <T>(value: T) => value` is a generic arrow; as TSX it is an unclosed JSX element, and the parser folds the rest of the file into the error node. A `RN.Linking.openURL` after one reported nothing, and so did the same call with its import above the arrow. The file name goes into the parse now and TypeScript reads the kind off the extension; `externalLinkOffenders` passes the real path, which it had all along. `ExportDeclaration` was never inspected, so `export { Linking } from 'react-native'` put the name back in reach of anything importing that module while the census saw an import list it was not on. All four shapes are read — named, renamed, `export *` and `export * as` — and reported at the export statement, which is the line to delete exactly as an import is. A re-export of another name, or of `Linking` from somewhere that is not react-native, stays unnamed. Seven fixtures. Five red on the previous walk: the `.ts` generic arrow and the four re-export shapes. The two that pass before and after hold the other half, that re-exporting is not itself the offence. The named-import and re-export clauses read `propertyName ?? name` through one helper rather than two spellings of it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-09-19 21:06:05 -04:00
Jinwoo HongandClaude	65cde9bb80	fix(mobile): name the source-control Back controls and split the dock's Close from Back (OTA phase C, C4.3) (#21739) ... * fix(mobile): name the custom-key drawer's Back for the accessibility tree (OTA phase C, C4.3) `CustomKeyModal`'s Back is a bare `Pressable` with a label and no role, so a screen reader has nothing to announce it as. It is reachable today from the session sheets and the terminal shortcut settings, so this is a gap now, not only inside the page. It surfaces here because `screenTree` takes a screen's directory: C4.4 registering the review route puts the whole of `src/components` under the Back rule. Fixing it in the PR that registers would make a route entry carry unrelated accessibility work, so the census gains a case that holds the arriving trees to the same two rules before the rows land. Red first it printed exactly what the rule would: src/components/CustomKeyModal.tsx:191 role=none label=Back Once `PAGE_SERVED_SCREENS` has the two rows, `CONTROLS` covers these trees and the new case becomes a second reading of the same thing; it says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): split the source-control header's Close from its Back (OTA phase C, C4.3) One `Pressable` served both modes — `onPress={onBack}` with a conditional label, `X` docked and `ChevronLeft` full-screen. The Back census reads a control by what its handler does, so it sees that one as a Back and then requires a label starting with "Back", in a mode where the control dismisses the dock beside the terminal. The cheap way to go green is to call a close "Back", which satisfies the rule by making the wording wrong. So the modes become two controls. Embedded presses `onClose` and is named "Close source control"; otherwise it presses `onBack` and is named "Back to session". Both carry the button role. The panel stops choosing by mode and passes both handlers; the dock keeps exactly the behaviour it had, its dismiss still `onRequestClose` falling back to a pop. Probed before it was written: run through the census's own predicate, the post-split shape yields one back control rather than two — `onClose` matches neither the handler pattern nor a declaration this file holds, being a destructured prop — so the Close is invisible to the rule and the presence precondition still holds on the Back. The component test is what the census cannot do. Nothing else pins this: no golden names this component and no parity census covers `src/source-control`. Red first against the single control, both cases failed on the missing role. And renaming the Close to "Back to close source control" — the gaming this split exists to prevent — reds the component test while leaving the census green, which is the whole argument for having both. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): assert a Back per arriving tree, and name the refresh control (OTA phase C, C4.3 round 1) The arriving-trees presence case counted controls over the union of both trees, so one tree answered for the other. Per tree now, in the shape the block above it already uses per screen module. Red first, with the mutation round 1 named: rename the route branch's handler to `onDismiss` and its label to `Return to session`, and the only Back in `src/source-control` disappears. The per-tree case names that tree. The same mutation against the union count passes all six cases, which is what the change is for. The refresh control had a label and no role, so react-native-web renders a `div` carrying `aria-label` and a screen reader announces no control. Its two siblings in this header already carry one. Two claims in my round-1 report were wrong and I am the reason the body carried them. The Close pressing `onBack` reds the census's label rule as well as the component test, not the component test alone; and no fixture of the post-split shape exists — the shape is read from the real file. Both were stated from reasoning rather than from a run. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert an arriving Back per screen, not per tree (OTA phase C, C4.3 round 2) Round 1 moved the arriving-trees presence assertion from the union to each tree, which was not far enough. `src/components` holds two Backs, so the tree answers for both: renaming `MobileDiffReviewHeader`'s handler to `onDismiss` and its label to `Return to session` leaves every case in the file green, with `CustomKeyModal.tsx:191` standing in for the screen that just lost its Back. Reproduced before the change — six passed with the review header's Back gone. Per screen module now, which is what the table above already does and for the same reason its docstring gives: a directory with more than one control cannot say which one a rule was written about. The two modules are named and the trees derive from them, so the pair C4.4 adds to `PAGE_SERVED_SCREENS` is the same pair spelled once here. Both mutations red on the new case and name the file: the review header's rename names `src/components/MobileDiffReviewHeader.tsx`, and round 1's source-control rename still names its own, so this does not trade one cover for another. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-09-19 20:51:47 -04:00
Jinwoo Hong	cef4416115	fix(mobile): name the narrow host header's controls and gate the drawer's hardware back on web (OTA phase C, C2.10) (#21729) ... * fix(mobile): name the narrow host header's controls The header renders two toolbars and the phone sees the narrow one, whose controls carried neither a role nor a name. A screen reader could not find them, and C2.9's render check could only assert their absence at 390 px. The wide toolbar already names every control from the same state, so the fix is to say the same thing rather than invent wording: filter, sort, group, accounts, tasks and the search toggle take their wide sibling's role and label expression verbatim. The census names a seventh site the plan did not: the Reconnect button in the status bar above both toolbars, which is shared rather than narrow and has no wide sibling. It announces through its Text child today, so it takes the role and the string it already renders. No layout, style, handler or order changed; the diff is accessibility props only. The census parses the file with the TypeScript API and rules that every Pressable carrying an onPress has a button role and a name, and that a control both toolbars render is named the same way in both. It keys the pairing on the handler, because that is what makes two elements the same control, and asserts each shared handler is found exactly twice, so a control deleted from one toolbar cannot leave the naming rule comparing a group of one with itself. Red first, naming all seven sites by path and line. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): stop the right drawer arming hardware back on web React Native Web logs "BackHandler is not supported on web and should not be used." and returns an inert subscription, so inside the shell's page every open of this drawer put that line on the console and armed nothing. The gate is the one mounted-bottom-drawer and the file preview already carry, with the same comment stating the degradation: there is no hardware back in a WebView, and the shell owns the one the phone has. The drawer had no render test. This one mocks react-native, the safe area, gesture handler and Reanimated the way the bottom drawer's hand-back test does, and reads the call rather than the console: on iOS and on Android the handler is registered once for 'hardwareBackPress' and released when the drawer hides, and on web it is never reached. The three native cases are the control that keeps the web case honest; they passed before the fix, which is what makes the single red meaningful. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): type the right drawer test's element helper The tests-typecheck ratchet was red on the previous commit: the drawer's props declare `children` as required, so passing it as createElement's third argument left no overload matching. It is a prop here, and the helper answers a ReactElement rather than a return type borrowed from createElement. Test files sit outside `tsc --noEmit`, so only the ratchet sees this; it is the gate that exists because a type-level pin in an unchecked test proves nothing. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): write the right drawer test in JSX Lint was red on the previous commit and I ran it in the same command as the commit, so it landed: passing `children` as a prop to satisfy the type checker is exactly what react(no-children-prop) refuses. The canonical form settles both, so the test is JSX in a .tsx file and the drawer takes its body as a child again. The StyleSheet mock's generic needs the trailing comma a .tsx file requires. Re-proved in this form: with the web gate removed the web case fails and the three native cases still pass. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): derive the header's naming groups, and read a spread as unknown Round 1, four folds. The drawer's comment claimed the console line was observed inside the shell's page. It was not: the drawer's one caller is the review screen, whose route C4 serves, so no page closure reaches it today. The gate is pre-emptive and now says so in its own words rather than borrowing the bottom drawer's sentence. The naming rule iterated a hand-written list, so it only ever compared the six controls both toolbars render. Giving the two `actions.openFloatingWorkspace` sites different labels left the census green. The groups are derived from the discovered controls now, keyed by the handler text, so any handler this header presses from more than one place is compared and the failure prints both names. The declared list stays as the precondition it always was: each of the six is found exactly twice, which is what keeps the derived rule from holding vacuously over a file with no repeated handler. The scan read `Pressable` only and dropped any control whose `onPress` read as empty text, which is what a spread reads as. It reads `TouchableOpacity` too now, and a spread answers unknown rather than absent: a control whose handler or whose accessibility props arrive through one is kept, fails both rules, and prints `spread` rather than `none`, so it can never be mistaken for a control the scan judged. Red first on all four: the reviewer's disagreeing-label mutation, a spread over the a11y props, a spread over the handler, an unlabelled TouchableOpacity, and a shared control deleted from one toolbar. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read a braced-empty label as unnamed, from one shared reader Round 2, two folds. Only a bare `""` or an omitted attribute read as unnamed, so `accessibilityLabel={undefined}`, `{''}` and an empty template all left a control with nothing to announce and the census green. Reproduced on both Tasks sites with each of the three shapes before the fix. The reader unwraps a braced expression now: a string or a no-substitution template answers its own text, and the identifier `undefined` answers empty, so all three read as unnamed. That reader was a near-verbatim copy in both censuses, which is how one of them could have gained this rule and the other kept the hole. It lives in one module under mobile-web-shell now, named for what it reads and typechecked by mobile tsc rather than by the ratchet alone. Both censuses import it and neither changed an assertion; their diffs are the deleted copies and the import. Red first, five mutations: the three empty shapes on both Tasks sites here, and `{undefined}` and `{''}` on the tasks Back, which the page census now catches too and did not before. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 18:33:20 -04:00
Jinwoo Hong	ac024d4f05	feat(mobile): serve the files explorer and preview from the page (OTA phase C, C3.1) (#21710) ... * refactor(mobile): take the files screens' router from the handoff seam Inside the shell's page a screen is one document standing in for one screen, so a target the page does not render has to be handed back to the app that does. `useRouteHandoff` is where that decision lives, and its web sibling is the only thing that makes it; both files screens held expo-router's own `useRouter`, so on the web the explorer's Back and the preview's Back would post nothing and a target outside the page would paint Unmatched over the page it is on. Natively this is the same object — `route-handoff.ts` is `useRouter()` — so no behaviour moves here, and `back()` stays expo-router's until the navigate-back verb lands and the seam starts wrapping it. A census rather than a behaviour test: neither screen's own tests can see the difference, because a push that is never handed off still works for a target inside the page. It walks this directory, refuses a value import of expo-router, and names the two screens that must hold a router so a walk that found nothing fails instead of passing empty. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): let the shell stand in for the two files routes Both route files take the index.tsx shape — flag, MobileWebShellScreen, native screen as fallback — and both gain the `.web.tsx` sibling that shape forces. Inert until the manifest lists these routes: the shell answers `native-route` for a route the bundle does not name, which is what `fallback` renders, and the flag is `__DEV__`-only besides. Listing them waits on C2.3 and C2.5. The sibling is not a precaution. The manifest defers every route behind `import()`, so a native-only route module is invisible until the page opens that route; the render check now opens both and, without the siblings, painted `expo-modules-core.requireNativeViewManager is not available on web` instead of the screen. That is also why the two cases render the route rather than asserting a file exists. The file path never becomes a path segment: only `hostId` and `worktreeId` are spelled into the pathname, encoded, and everything else — `relativePath`, `absolutePath`, `cwd`, `pathText` — is a param, which is how a `/`, a space or a `..` stays out of the segment vocabulary the bridge holds a route to. The preview render case proves the round trip on `docs/my notes/readme.md`. `mobileFilePreviewShellParams` drops a param the normalizer left `undefined` rather than sending it empty, because the page reads these back through useLocalSearchParams where `line: ''` and no `line` are different screens. Its test drives the normalizer rather than a hand-written literal: the literal omits the key entirely, so it held with the filter removed. The preview case also records what React Native Web says out loud — BackHandler is inert on web, so Android back inside the page skips the unsaved-draft prompt. Named in the assertion rather than filtered out, so closing it is a change to that line. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): ask about an unsaved draft in the screen, not through Alert React Native Web's `Alert` is `static alert() {}`. Inside the shell's page that made Back with an unsaved terminal-artifact draft a button that did nothing at all: no prompt, because the dialog is a no-op, and no navigation either, because the code took the branch that shows one. Silently, with nothing on the console. The prompt is now a row under the header. Not `ConfirmModal`, which every other confirm here uses: that is a `BottomDrawer`, and C1.9 has Reanimated's animated styles never reaching the DOM node on WKWebView, so on iOS in the page the drawer parks off-screen and Back would be dead a second way. This paints the same on every platform with no animation behind it. Hardware back is registered natively only. React Native Web's `BackHandler.addEventListener` logs "BackHandler is not supported on web and should not be used." and hands back an inert subscription, so the guard never armed there regardless; the render check asserted that console error on main and now asserts none. The degradation is real and stated rather than hidden: Android back inside the page pops the native stack without asking, and the page's own Back control is where the question lives. The decision moved to a hook so it is testable without a screen: the prompt also drops itself when the draft it was about is saved or reverted, which is a state `Alert` had no way to be in. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep expo-haptics' DOM shim out of the page expo-haptics has a web build, and with no `navigator.vibrate` — iOS Safari, which is the WebView the page runs in — it fakes a haptic by appending a hidden `<label><input type="checkbox" switch>` to `document.head`, clicking it, and removing it, once per call. C1.9 traced a long press that never fired on the worktree list to exactly that stray click, and the file explorer calls `triggerSelection` on every row tap, so C3 is the first domain to fire it per tap rather than per long press. `haptics.web.ts` answers the same five names with nothing. A phone holding the page is a phone whose native app is right there with the real haptics, and a missing tap feedback is worth less than a tap that does not register. The test reads the shipped bytes rather than the import, because that is the claim: with the override removed the bundle carries `ariaHidden` and `pointer: coarse`; with it, neither, nor the `setAttribute("switch"` that does the clicking. Not `navigator.vibrate` — react-native-web's own Vibration export calls that and touches no DOM until something invokes it, which cost this test one wrong red before it was narrowed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the files routes native when the page could not be given one A file path is a param, so `/`, spaces and `..` all cross safely — but `BRIDGE_MAX_ROUTE_PARAM_CHARS` is 1024 and a Windows long path is not bounded by anything the user cannot exceed. The symptom is not the blank document the design predicted, and the correction matters: `bridge-host.ts` already parses the route against the page's own schema and drops it to `null` when it fails, so `init` arrives naming no screen and the page paints "Update Orca to open this workspace" — a wrong message about a fine app, over a native screen that works. Deciding before the switch instead leaves the route native, which is where every route starts. The schema is the predicate rather than a copy of its bounds, so the rule cannot drift from the half that matters, which is the half the page reads. The same call also refuses a `worktreeId` the segment rule will not route: `..` survives `encodeURIComponent`, which is the C1.8 class. The tests assert the schema really refuses each input before asserting the guard does, so neither case can pass by being impossible. This belongs in the shell beside the schema; it is in the files domain while the contract files are the C2 lane's. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin what keeps a file path out of the route vocabulary Seven shapes, one case each rather than a representative: a plain path, a space, a dot segment, an already-encoded slash, a fragment, non-ASCII, and an absolute path. Each is checked in the two directions a path travels — the href the shell writes into the page's history, and the href the page would hand back — for both the pattern accepting it and the path coming back out of the query unchanged. The counterfactual is in the file: the same paths spelled as a segment are refused. Without that, the cases above would hold for a rule that was never doing any work. Mutating `stringifyRouteHref` to join its query by hand instead of through `URLSearchParams` fails three of them. Also fixes two new test files the tests-typecheck ratchet caught: the partial `react-native` mock needs a typed `addEventListener`, `act` will not take a callback that returns a value, and `findAllByType('Pressable')` does not typecheck against `ElementType` — the neighbouring files that do it are grandfathered, so the tag comparison goes through a helper instead. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): derive the discard prompt instead of clearing it in an effect Both changed-code gate findings, which the lane had not run until the last commit. React Doctor is right: the effect that cleared the prompt when the draft went away adjusted state after a prop changed, so a save landing while the prompt was up painted one frame still offering to discard nothing. The prompt is now `asking && hasUnsavedDraft`, which cannot be stale by construction, and the test that covers it passes unchanged. The hoisted mock's `as` on a string literal is gone too: the literal narrows on its own and the tests reassign it, so the holder is annotated instead. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): add the files routes to the hybrid shell flag census The census pins every file that reads `useMobileWebShellEnabled`, because a reader nobody listed is how a dark feature stops being dark. C3's two routes are deliberate entries: each has a native screen behind it as `fallback`, and each is inert until the manifest lists the route. Found by the full mobile suite rather than by the files subset this lane had been running per commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): serve the files explorer and preview from the page The last C3 commit: both routes join MOBILE_WEB_PAGE_ROUTES, and the shell starts rendering the page for them on a phone with the dev flag on. Grants are not the same for the two, and the difference is the point. Both take `navigate` (Back pops the native stack, and the explorer's rows open the preview beside it) and `storage` (the shared components the host layout renders above them). Only the preview takes `externalLink`: a Markdown preview renders links and `MobileMarkdown` opens them through the platform seam. The explorer does not, and measuring is what says so rather than reading. Every page route reaches `external-link.web.ts` — `/h/[hostId]` and agent-history included, both granted nothing for it — because the protocol wall in the shared host layout imports it. So closure membership is not the oracle for a grant; the question is whether the route's own screens call it, and only the preview's do. `MobileMarkdown` is in the preview closure and absent from the explorer's, which the census now asserts in both directions. Neither route writes a clipboard, so neither takes `native.clipboard.write`; the census pins that as the absence of both `ExpoClipboard.web.js` and the clipboard seam, with the tasks closure as the control that the probe can see one when there is one. The seam predicate moved into a module both censuses import rather than being restated per series: two spellings of one rule drift, and this one is a regex. Red-first: both manifest assertions failed on the new entries before they were updated, and routing `MobileMarkdown` around the seam fails the preview's census while leaving the explorer's passing, which is the asymmetry the grants encode. Closure sizes as the page ships them, extensionless so the `.web.tsx` is what is measured: explorer 3439 modules / 302 local / 10 under src/files, preview 3667 / 331 / 20. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read the files route's ids as one value and key the shell on them Two round-1 findings, both reproduced before the fix. A repeated query key reaches `useLocalSearchParams` as an array, and the explorer read `hostId` and `worktreeId` bare. `String(['a','b'])` is `a,b`, so the template built `/h/host-a%2Chost-b/files/wt-1%2Cwt-2` — a single segment the bridge's rule accepts, and the shell would open a page for a host nobody has. Read through `firstParam` now, as the tasks and agent-history switches do. The preview already went through `singleParam` and is unchanged. Neither switch keyed `MobileWebShellScreen`, where `index.tsx`, `tasks.tsx` and agent-history all do. A host captures the grants its session opened with, so a screen reused across a route change keeps authorising frames under the grants of the route the page has left; only a remount drops that bridge. Both are keyed on the route pathname now, with agent-history's reason. The new route test is the agent-history one's shape. It caught both: the array case landed on no route at all, because `name` was an array too and the schema refuses a non-string param value, and the two lifecycle cases saw a prop update where a remount was owed. It also needs agent-history's `lucide-react-native` mock, since `firstParam` lives in the source-control barrel. `name` is now omitted when empty rather than sent as `name=`, matching the two switches beside it: an absent label lets the panel derive its own. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): confirm a discarded draft with the app's own modal Round-1 findings 3, 4, 5 and the minor one. **ConfirmModal, not the bespoke row.** The row existed because C1.9 had Reanimated's animated styles never reaching the DOM node on WKWebView, which left every BottomDrawer parked off-screen. C1.10 (`b7c06900e2`, an ancestor of this branch) fixed that with a dependency array on the mapper hooks, and the drawer render check now holds it on WebKit as well as Chromium. With the reason gone the row does not stand on its other merits: `Alert.alert` was modal on native before the page existed, and the row quietly changed that for phones too, so the app's own confirm is both the idiom and the closer behaviour. `MobileFilePreviewDiscardPrompt`, its test and its thirty style keys are gone; the hook's state machine and its tests are unchanged. **The encoding test claimed more than it pinned.** Hand-joining the query reds only three of the seven shapes; `docs/readme.md`, `../etc/passwd`, `docs/日本語.md` and `/logs/run.txt` are encoding-neutral in the query, whose pattern half is `[^#\s]*` and admits a slash, a dot segment and non-ASCII verbatim. Rather than narrow the claim in a comment, the split is now pinned by behaviour: each neutral shape must survive the query unencoded, each load-bearing one must not. Moving `docs/readme.md` between the lists fails it. **The manifest comment named one shared-layout opener and there are two.** The New Workspace source field, which the sidebar renders on a wide layout, opens a URL through the seam as well. Both are the shared layout's and every `/h` route reaches both, `/h/[hostId]` included with no `externalLink`, so the tablet tap is dead on all of them — recorded here as pre-existing rather than fixed, since the grants do not move. **Minor:** the dot-segment case in the guard test now asserts the schema refuses the route before asserting the guard returns null, as the length case does. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): stop every page drawer logging a BackHandler error when it opens Round-2 findings. **The registration belongs to the drawer, and that is where the guard went.** `mounted-bottom-drawer.tsx` armed `hardwareBackPress` whenever a drawer was visible and interactive, with no platform check, so the hook's claim to have dropped that console line held only while its prompt was closed — and every page drawer since C1 has logged it on open. Platform-gated at the drawer now; the hook's comment says so rather than claiming the credit. **Nothing had ever opened a modal in a browser.** The render check next door mounts both files routes and reads what they paint but taps nothing, so `ConfirmModal` inside the page — a BottomDrawer, so Reanimated, a portal and a gesture handler — was unproved. A new render file loads an editable terminal artifact through the harness's scripted reply, edits it, taps the page's Back, and asserts the prompt's title is up and no BackHandler line is on the console. Red first on exactly that line; the prompt itself painted, which is also the first proof on a browser that C1.10's fix carries a real drawer in the page. A second case answers Stay and checks the draft survives. Its own file rather than the render check's, which is at 482 of the 600-line cap; registered in pr.yml. **The encoding rule was stated wrong.** Two rules decide it and neither is about paths: the pattern's query half refuses whitespace and `#`, and `URLSearchParams` is form-urlencoded, so it reinterprets `&`, `+` and a valid `%XX`. `a+b.ts` reads back `a b.ts` and `a&b.ts` reads back `a`, so both are load-bearing; `a=b.ts` and `a%b.ts` are not, because only the first `=` splits the pair and a lone `%` begins no escape. A newline joins the load-bearing list as the refused shape rather than the altered one. **The web sibling read its params bare** where the native one uses `firstParam`. Not reachable — the page only arrives through `init.route`, whose params are already `Record<string, string>` — but the two files are meant to be one screen. The preview keys on the pathname alone, and the comment now says why that is enough: every caller in this tree pushes. Closures after this: explorer 3441 / 304 / 10, preview 3666 / 330 / 19. The explorer grew two modules because its web sibling now reaches `firstParam`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): give the explorer the grants the preview needs, and key on the route Bot findings, one of them a real gap. **Pullfrog is right, and my grant oracle was half a rule.** Grants resolve once, from the route the shell opened: `grantsForRoute` reads `session.routePathname` and `init.grants.native` carries the answer for that session. The explorer's rows push to the preview, and because the preview is a page route that push stays inside the same document — no second `init`. So a preview opened that way runs under the explorer's grants, and a Markdown link in it was refused by `notifyExternalLink` with nothing on screen to say why. "Does the route's own screen call it" was right for a route's own screens and wrong for the routes it reaches in-page, so the explorer now declares `externalLink` as a transitive grant, with the comment saying that rather than claiming it opens links. The census pins the pair as a superset; removing the grant reds it. **The seam regexes matched one quote style.** A double-quoted `react-native` specifier walked past both censuses unseen. Both styles now, with the predicate tested directly for the first time. **The discard request outlived its draft.** `asking` stayed set after a save or a revert, so the next edit re-showed the prompt with no Back request behind it. The request is now dropped when the draft it was about goes, adjusted during render rather than in an effect — the shape React Doctor named in the round-1 fold. Red first: save with the prompt up, edit again, prompt is back. **CodeRabbit's keying comment is a correctness point, not the question I answered.** The page learns its route exactly once, out of `init`, so a same-path param change — another file in the same worktree — left the shell mounted and the page still showing the file it was opened on. My comment claimed "the screen reloads the preview from the param either way", which is true only with the shell absent. Both switches key on the whole route now, params included; two tests cover the same-path case and both red on a pathname-only key. `build-mobile-web-app-bundle.test.mjs` hit 601 of its 600-line cap on the way, so the two manifest assertions now share one expected list instead of repeating it. Closures unchanged: explorer 3441 / 304 / 10, preview 3666 / 330 / 19. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make the seam test import the module it is testing Round 3. **The blocker is mine and the reviewer's diagnosis is exact.** The seam predicate test imported an absolute path into this lane's worktree. On CI that module does not exist and it takes the whole `config/scripts` suite down; here it resolved to the same file by accident, so the test was green against a tree rather than against the checkout — which is why reverting the double-quote fix left it passing and the predicate untested. Relative now, and proved: reverting the fix in place reds both double-quoted cases, which is the first time this test has failed for the right reason. Every file this PR touches is grepped for `/Users/` and `orca-lanes`; none carries a path. **Three comments outlived the grant change.** The two lists became equal when the explorer took `externalLink`, so "longer than the explorer's" and "declared with different grants" were both false. Corrected to what is actually true: the lists are equal and the reasons are not — the preview has its own consumer in `MobileMarkdown`, the explorer has none and declares the grant because its rows push to the preview in-page. **The duplicated serializer is pinned rather than imported.** `shellRouteHref` lives in `page-bootstrap.ts` beside the page's RPC client and its document channel, so a native route file importing it would pull both into the app. The copy stays, and a test asserts the two agree on three routes; dropping the empty-search branch reds it. **Recorded, not fixed:** the sidebar `HostScreen` pushes to `/h/<id>/tasks` through the handoff, which is local, so on a tablet the tasks page runs without `native.clipboard.write` from any page route and its copy actions refuse silently. Pre-existing since C2.1 for the worktree list and agent history. Named in the explorer's manifest comment as the known remaining hop, with the fix being a handoff rule in its own PR. The equality pin needed `it.each<BridgeInitRoute>`: the inferred table is a union whose members carry `?: undefined`, which the ratchet caught. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 15:47:25 -04:00
Jinwoo Hong	b6e8b1a7b2	feat(mobile): serve the tasks screen from the page, with its seams (OTA phase C, C2.1 + C2.5) (#21694) ... * fix(mobile): encode the host id in the tasks workspace-creation href (OTA phase C, C2.1) `use-mobile-tasks-workspace-create-actions.tsx` built `/h/${hostId}/session/...` with the host id interpolated raw — the C1.2 class. A host id carrying `/`, `#`, `?` or whitespace reaches the wire as an href `BRIDGE_ROUTE_HREF_PATTERN` refuses, the handoff falls through to the local router, and expo-router's Unmatched paints over the page. Deleted rather than patched: `hostNewWorktreeSessionRoute` already builds this exact href with both segments encoded, and already has the test that pins it. The screen now calls it. The census that caught it stays: no module under `src/tasks` may interpolate into `/h/${...}` without encoding, which is the rule rather than this one line. Three refactor-parity hashes move with the statement change and are recorded in that file the way every earlier movement is. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): route the tasks tree's external links through the seam (OTA phase C, C2.1) Ten of the twelve call sites in the tasks page closure: the nine under `src/tasks`, swapped by one export in the dependency barrel, and `MobileMarkdown.tsx`, which imports react-native directly and is edited in place. Inside the shell's WebView react-native-web's `openURL` calls `window.open(url, '_blank')`, which both shells refuse — iOS returns nil from `createWebViewWith`, Android false from `onCreateWindow` — and resolves regardless. Every one of these sites would have reported success into a tap that opened nothing. The barrel's `Linking` is typed `{ openURL: (url: string) => void }`, so a `.catch` on it is a compile error rather than a handler for a rejection that cannot arrive; the seam names its own failures. `MobileMarkdown`'s own `.catch(() => {})` goes with the swap for the same reason. No parity hash moved: the barrel and `MobileMarkdown` are outside the refactor-parity family's source set. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): route the shared screens' external links through the seam, with a census (OTA phase C, C2.1) The last two of the twelve call sites in the tasks page closure: `ProtocolBlockScreen.tsx` and the `openExternalUrl` prop wiring at `host-screen-overlays.tsx`. Both are shared with native routes and with the already-live `/h/[hostId]` page, so this changes that page too: its external links go from the measured `window.open` no-op — which both shells refuse and which resolves anyway — to a URL handed to the shell. Nothing changes on a phone, where the seam is `Linking.openURL` unchanged. The `openExternalUrl` prop chain is retyped `(url: string) => void` with it, and `SmartWorkspaceSourceField`'s `.catch(() => {})` goes: the seam names its own failures and never rejects, so that was a handler for a rejection that cannot arrive. The census is the rule rather than today's twelve sites: no module in the tasks page closure may reach react-native's `Linking`, by name or through a namespace import. It reads the closure from a new builder export — `metafile.inputs` for `_layout` plus the route, which is one definition of what a page contains — and checks which module the name comes from, not which text a call site writes, since the tasks tree still calls `Linking.openURL` and that `Linking` is now the barrel's seam-backed export. Confirmed to discriminate: restoring one react-native import turns it red. A second case pins that the seam is in the closure, so an empty offender list cannot also mean a page that reaches no link code at all. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): write the tasks clipboard through the shell's verb (OTA phase C, C2.1) The two `Clipboard.setStringAsync` sites in the tasks page closure move onto a seam, `src/platform/clipboard.ts` with a `.web.ts` sibling, registered in the overrides. A hook rather than a function because the web form needs the page's bridge client, which is React context. Native is `expo-clipboard` unchanged. Web calls `native.clipboard.write` through `useNativeVerbs`, because `expo-clipboard` on the web is `navigator.clipboard` and needs a secure context: the iOS shell serves the page from a custom scheme and Android from `https`, so that path would work on one platform and silently not on the other, with nothing at the call site able to tell. Both seams reject rather than return false, and both call sites already wrap the write in a `catch` that puts the message on screen — so a write that did not land says so instead of showing "Copied". A route that has not declared `native.clipboard.write` is refused before a frame is sent and lands in that same `catch`; the route declares it in the entry commit. Two parity hashes move, the hook list and the statement hash, each by one entry, and are recorded in that file. `semantics` holds, as do render and style: no RPC call, method literal or JSX host signature changed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): hand the tasks Back button to the shell (OTA phase C, C2.1) The tasks header's `router.back()` reached expo-router through the dependency barrel, and inside the page that moves nothing: the document holds the single history entry the entry wrote with `replaceState`. The stack with somewhere to go is the native one the shell pushed the page onto. One line in the barrel, as with `Linking`: `useRouteHandoff` is router-shaped, so every call site is unchanged. On a phone it is expo-router. Inside the page it keeps a route the page renders and posts `navigate-back` for a Back the document cannot serve — the C2.2 seam, which until now had no consumer. No parity hash moved: the barrel is outside the refactor-parity source set, and no call site changed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): render mermaid as its own source box on the web (OTA phase C, C2.5) `MermaidDiagram` is in the tasks page closure, reached through `MobileMarkdown`, and it renders the diagram inside a sandboxed `WebView`. `react-native-webview` is a native component with no browser counterpart: importing it runs a codegen lookup that throws, and the route manifest imports every route, so one such import takes the whole page down rather than one diagram. The web sibling renders the labelled source box the native component already falls back to on a parse or render error, with that component's own styles, so the degradation looks like a state the product already has rather than a second design. Not a browser renderer, and the reason is not reach: mermaid is a browser library and the engine bundle is vendored. It is that the native path's safety comes from the WebView it runs in — `buildHtml` escapes `</script>` and the U+2028/U+2029 separators because diagram source is untrusted agent and PR content — and a DOM path has no such sandbox, so it needs its own escaping and its own proof. That is a change of its own, not a smaller version of this one. Registered in the overrides, whose gate fails on an unlisted `.web.*` file. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): turn the tasks route on for the page (OTA phase C, C2.1) The entry: `/h/[hostId]/tasks` joins `MOBILE_WEB_PAGE_ROUTES`, the route file becomes the shell's flag switch in `index.tsx`'s shape, and a `.web.tsx` sibling renders the screen directly, registered in the overrides. The screen moves to `src/tasks/MobileTasksScreen.tsx` first, verbatim — body byte-identical, imports rewritten to `./`. It has to: under the builder's `resolveExtensions` a web sibling importing `./tasks` resolves back to itself, which is why every other shell route's screen already lives in `src`. The parity family follows the file rather than the path. `TASKS_ROUTE` leaves `MOBILE_TASKS_SOURCE_FILES` — `SOURCE_PATTERN` already matches `MobileTasks*.tsx`, so listing it too would double-count — and the execution reader points at the new file. Measured rather than predicted: all six refactor-parity cases pass unchanged. No hash moved, including the family text and declaration list, because the new name sorts where the route path sat. The route declares `navigate`, `storage`, `externalLink` and `native.clipboard.write`, which the grammar fold made expressible and per-route scoping makes meaningful: it is granted those and not the rest of what this shell implements. The browser check covers what only a browser answers — every module in the closure evaluating under React Native Web, `taskSource` surviving the handshake into the page's own URL, and the route's chunk arriving on a client-side navigation. It states plainly what it does not cover: the three seams are reached from controls that need provider data the double does not serve, so a case posting those frames directly would prove the transport and read as a tap it never performed. Both new checks join the `mobile_web_app` job. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(config): resolve a route closure the way the bundle ships it (OTA phase C, C2.1) `mobileWebAppRouteClosure` took the route's explicit `.tsx` path as an entry point, so esbuild used that file directly and `resolveExtensions` never ran. For a route with a `.web.tsx` sibling that measured the native switch, which no browser loads: the tasks closure came back carrying `MobileWebShellScreen`, and with it a `Linking` import the census then reported as an offender. Extensionless now, so the closure is the one the page actually contains: 3775 modules, 428 local, with `external-link.web.ts` and `clipboard.web.ts` in it and the shell screen out. The route-manifest pins move with the tasks route joining `MOBILE_WEB_PAGE_ROUTES`, in both the declaration check and the built manifest. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): cover the clipboard seam, close two page escapes, share the mermaid props (OTA phase C, C2.1) Four from round 1. The clipboard seam shipped untested. Both halves have one now: the native form rejects when `setStringAsync` answers false and resolves when it does not, and the web form is driven through the real port pair — resolving on a reply, rejecting when the shell says the pasteboard refused, and rejecting on an ungranted route without putting a frame on the wire. The tasks barrel still re-exported `expo-clipboard` with no consumer, which kept `ExpoClipboard.web.js` — the `navigator.clipboard` path this series exists to avoid — inside the page closure. Deleted, and asserted as the module's absence from that closure rather than as a count of importers: a new import puts the file back whoever writes it. `ProtocolBlockScreen` reached expo-router's singleton for its way out to the host list. A singleton is the one shape the handoff cannot intercept — it is not a hook, so the page's bridge client is never consulted — and `/` is a route the page does not carry, so inside the shell that replace rendered the root route in the WebView instead of leaving it. Pre-existing and live via `/h/[hostId]`; routed through the handoff now. Two suites' `expo-router` mocks gain the hook the handoff reads. `MermaidDiagram.web.tsx` redeclared its props; it imports the native component's type, so drift fails tsc. No parity hash moved: none of these files is in the refactor-parity source set. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(config): use endsWith for the clipboard module check The changed-code gate refuses a dollar-anchored regex where `String#endsWith` says the same thing. No behaviour change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): close the href census gap, read route params through firstParam (OTA phase C, C2.1) Five from round 2, two of them real. The raw-interpolation census inspected only the leading `${...}`, so `` `/h/${encodeURIComponent(hostId)}/session/${worktreeId}` `` passed it — and a worktree id carrying `/`, `#`, `?` or whitespace breaks the href exactly as a host id does. It now refuses any hand-built `/h/...` template with any interpolation left raw, whichever segment it is. Proved against exactly that shape in a throwaway before the change, which the old rule admitted. The tasks switch read `hostId` and `taskSource` as plain strings. expo-router hands back an array for a repeated query key, so a duplicate `?hostId=` built `/h/host-a%2Chost-b/tasks`; both go through `firstParam` now, as the agent-history switch does. `index.tsx` is untouched, per the Phase D list. Three in the render check's prose: the header claimed the browser proves the three seams fire from a tap, which the file's own closing note denies; a module count repeated a number the closure test already pins; and a `replies` parameter was threaded through without ever being supplied. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 13:40:26 -04:00
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	2026-09-19 03:47:44 -04:00
Jinwoo Hong	57fdf68ab3	feat(mobile): let the page hand a link to the device through the shell (OTA phase C, C2.3) (#21597) ... * feat(mobile): answer externalLink on the shell side of the bridge (OTA phase C, C2.3) A page has no way to open a URL outside itself: `Linking.openURL` is the app's, and inside the shell the page is a document that cannot reach it. Adds `notify { name: 'externalLink', url }` and a new grant name of its own in `MOBILE_WEB_SHELL_GRANTS`, rather than a verb of `navigate` — `navigate` opens a screen this app carries, this hands a URL to whatever the device opens it with, and a shell implementing one and not the other is a real shell the route policy has to be able to describe. `https:`, `http:` and `mailto:` only, and broad inside that: any host, any path, because a grant that named GitHub would grow a row per provider. The rule is parsed rather than prefix-matched, since a scheme is what a URL parser says it is and `startsWith('https:')` reads one out of `javascript:alert("https://x")`. It is enforced at the frame as well as at the page's call site, so a page that skipped its own check still cannot reach the device handler. Bounded by the route href cap, per the ruling. `BRIDGE_PROTOCOL_VERSION` is not bumped. Inert until a consumer exists: no call site and no barrel is touched here. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): plumb externalLink through the bridge hook probe (OTA phase C, C2.3) The hook's own suite builds its caller options inline, so the new required option made it stop typechecking. `tsc -p tsconfig.json` excludes test files; only the tests-typecheck ratchet saw it. Adds the case that goes with it: a URL the page hands over reaches the caller that can leave the app, and nothing reaches the navigate path. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): let the page post an externalLink over the bridge (OTA phase C, C2.3) `notifyExternalLink(url)` on the page client, gated on the `externalLink` grant and on the same scheme rule the frame enforces. Checked twice on purpose. Nothing crosses back for a notify, so the boolean is the only answer a tap gets: a page that posted a URL the shell's reader then dropped would report "opened" into a frame nobody acted on, which is precisely the dead tap the grant exists to rule out. False before `init`, false after `close`, and never a throw — the callers are tap handlers with no catch around them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): add the external-link seam the tasks call sites will use (OTA phase C, C2.3) One module with a `.web.ts` sibling, which is the shape every platform gap in this bundle already takes. Native is `Linking.openURL` with the rejection swallowed, because every caller is a tap handler and `openURL` rejects for a URL no installed app claims. Web posts the `externalLink` notify after the same scheme check the frame enforces, names its refusals and throws nothing. The opener is published by the entry rather than read from context, for the reason `publishPageStorage` is: the callers are plain functions in render trees the provider does not wrap. A document that published none refuses every URL, which is the right answer for a page with no shell. Registered in `web-overrides.json`: inside the shell's WebView, react-native -web's `Linking.openURL` opens the URL in that WebView and replaces the page rather than handing it to the system browser. No call site and no barrel is touched. The consumer PR swaps them onto this. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state the real reason the page cannot open its own links (OTA phase C, C2.3) The override reason claimed react-native-web's `Linking.openURL` opens the URL in the shell's WebView and replaces the page. It does not. Read from react-native-web 0.21.2: `openURL` calls `window.open(url, '_blank', 'noopener')` and resolves whether or not anything opened; only a `tel:` URL assigns `window.location`, and none of the three allowed schemes is one. The true failure is the worse one and the better argument for the verb. Both shells refuse `window.open` outright, measured in their own sources: iOS sets `javaScriptCanOpenWindowsAutomatically = false` and returns nil from `WKUIDelegate`'s `createWebViewWith`; Android sets the same flag false, calls `setSupportMultipleWindows(false)` and returns false from `onCreateWindow`. So nothing opens, `openURL` resolves anyway, and the native path reports success into a tap that did nothing — precisely the dead tap the grant exists to rule out. The seam's own comment now says the same, so the two files cannot drift. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): report a URL the phone could not open (OTA phase C, C2.3) The shell swallowed `Linking.openURL`'s rejection. Nothing crosses back to the page for a notify, so an open that failed — a `mailto:` on a phone with no mail account — was silent on both sides. That is the one dead tap this verb does not rule out, and it was the only one with no record at all. Warned with the URL and the error, in the shape the two neighbouring reports in this screen use, and still not rethrown: this runs on the native frame handler. The contract now asks for the report rather than only for the absence of a throw. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): forward the URL the parser read, not the string the page sent (OTA phase C, C2.3) The scheme check reads the protocol through the WHATWG parser, which strips tab, LF and CR from anywhere in a URL and trims leading C0 and space before the scheme is visible. So `ht\ntps://example.com`, `https://example.com/a\r\n`, ` https://example.com/a ` and `https:example.com` all passed the check, and both sides then forwarded the original string. Not a scheme escape — the parser had already decided the scheme — but the device handler was given a URL the check never looked at, which is a dead tap through an allowed URL. `readBridgeExternalLinkUrl` answers the parsed href, and the page posts it and the host forwards it. Normalizing rather than comparing, because `https://example.com` differs from its own href by a path slash: refusing what differs from its normalization would refuse an ordinary URL. The cap now applies to the normalized form as well as the raw string, since percent-encoding expands and a string inside the cap can leave it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): exercise the seam's default opener instead of a published one (OTA phase C, C2.3) The case named for a document that published no opener published one first, and `post` is module state every earlier case had already set, so the default at the top of the module was never the thing under test. The only assertion was `not.toThrow()`, which passes against any implementation. `vi.resetModules()` and a fresh import, and the refusal reason is asserted. Confirmed to discriminate: flipping the default to `() => true` turns this case red and leaves the other three green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): build the openURL rejection per call, not at mock setup The case added for a failed open used `mockReturnValue(Promise.reject(failure))`, which builds the rejected promise at setup time. Nothing attaches a handler until the notify frame arrives several awaits later, so the suite reported an unhandled rejection and exited 1 with every test passing — a red run that reads as green in the counts alone. A fresh rejection per call closes the window, and `openUrl` is reset between cases so the mock cannot leak into one that does not expect it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): guard the openURL failure that escapes a tap handler (OTA phase C, C2.3) `Linking.openURL` validates before it returns anything: `_validateURL` is an `invariant` that throws for an empty string (react-native 0.83.10, `Libraries/Linking/Linking.js:117-123`). So the seam's `.catch` was attached to a promise that, in that case, never existed, and the throw went straight through a tap handler — contradicting the module's own claim to be safe in one. Both failure modes are now caught and reported, and neither is rethrown. The test double validates the way the real module does, because a mock that only rejects cannot reproduce the failure that escapes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): collapse the duplicate openURL wrapper onto the seam (OTA phase C, C2.3) `mobile-pr-url.ts` was the seam's native body already, byte for byte, written before it. It now re-exports the seam under its own name, so the empty-URL guard and the failure report reach its four callers too. Nothing changes natively: the seam's native form is what those callers were running. On the web they would now post the notify instead, which is the behaviour they should have had; it is unreachable today, since none of them is in a page route's closure. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read externalLink end to end over the port pair (OTA phase C, C2.3) The pair harness grew `externalLinks` and nothing read it. This is the `navigate` twin's shape, over the six normalization inputs: what the page put on the wire and what the shell forwarded are the same strings, read back off the frames rather than recomputed, and nothing reaches the shell's client. Two halves on purpose. The page normalizes before it posts, so over the client the host only ever receives an already-normalized URL and forwarding it raw would pass — the frame injected straight into the host at the end is what holds the host to the rule on its own. Confirmed to discriminate: reverting the host to forward `message.url` turns that assertion red. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile): match the neighbours on three small inconsistencies (OTA phase C, C2.3) Three of a kind, none behavioural: - the notify guard's docstring had a 109-character line; reflowed - the shell's could-not-open warning passed three arguments where the three other reports in that screen pass two; it now passes `{ url, error }` - the native seam's suite built its rejection with `mockReturnValue`, which is the shape `52191bab02` removed elsewhere; it now builds one per call, through the same double that validates the way the real module does Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 03:33:41 -04:00
Jinwoo Hong	889c2b562f	feat(mobile): say why a host is unreachable instead of "Connecting via Relay…" (#21566) ... * feat(mobile): say why a host is unreachable instead of "Connecting via Relay…" The home-screen host row and host header showed "Connecting via Relay…" for as long as the desktop stayed unreachable, even when every relay dial had ended with the cell's 4404 host-offline close. A user's diagnostics export showed 25 such dials over 25 hours behind that label, and the diagnostics report itself said "No single failure cause" because relay dial failures were not recognised and every app resume emptied the evidence window. Relay close codes now map to a closed RelayHostReachability verdict (signed-out, host-offline, credential-refused, unreachable), latched after two consecutive identical dial failures and cleared only by an authenticated session. The existing signed-out close reason becomes a member of the same verdict instead of a parallel boolean. classifyConnection renders each verdict as a label plus a detail line ("Host 1 is offline" / "Check it's awake, Orca is running, and you're signed in"). Relay dial failures carry their close code as a structured field on the connection log entry, so the diagnostics analysis names the cause without parsing error text, and an app resume no longer hides the last failure: it is reported with a "Before the app last resumed" qualifier and is never a sendable incident. * test(mobile): re-record RPC goldens at the new baseline Only header lines change: the baseline pin on every golden and the adapterSha256 on the twelve goldens whose mount adapters gained the getRelayHostReachability context method. No checkpoint moved, which also shows the commits between the old and new baseline changed no observed RPC behaviour. * fix(mobile): tell a refused relay credential to re-pair, not to find the same network A direct session also rotates the credential, but telling the user to connect on the same network once explains the mechanism instead of giving an action, and re-pairing is the one remedy that works from anywhere. * fix(mobile): let the newest relay failure win the diagnosis, and name the real stale boundary Relay-path evidence still outranks a newer direct timeout, but among relay failures the newest now wins: an older 4404 verdict no longer hides a newer session close (which was also the sendable incident) or a director refusal. The stale prefix names a network change when that, not a resume, was the boundary.	2026-09-19 00:24:25 -04:00
Jinwoo Hong	e2afb5eef9	feat(mobile): the page reads this host and keeps the app's pins (OTA phase C, C1.4) (#21503) ... * feat(mobile): the page mounts on the shell's init, with the client injected (OTA phase C, C1.1) The Route A entry built no client and mounted the route tree immediately, so the web provider minted its own: it read the page channel, built `BridgeRpcClient` and fell back to a placeholder that rejected every call. A tree that mounts before `init` reads synchronous getters against a client that knows no host, no state and no build, and the first render it records is the wrong one. The entry now owns the page's one client. It builds it from the channel at module scope, mounts nothing until `onReady` fires, and stamps the session and build ids `getShellSession()` returns on the document beside the mount state, so a screenshot, the render check and a device console read the same three facts. `client-context.web.tsx` takes that client by injection and serves it from `acquire()` for every hostId, because the bridge protocol names no host; the placeholder and its `BridgeTransportUnavailableError` are gone, along with the entry that pointed at them in the unvalidated-port inventory. A document with no channel is not inside the shell, so it says `unbridged` and stops rather than waiting out a backoff nobody answers. The render check gains a shell double that answers `ready` with `init`, reads the stamped session back off the document, and proves the gate is real by opening the same route with no double and finding an empty `#root`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): the shell names the screen, and the page routes to it (OTA phase C, C1.2) The shell serves its document at `/` and refuses every other path, so the page's own location matches no route in the tree it carries and expo-router paints Unmatched. Nothing in the document can tell it otherwise, so the screen has to cross the bridge. `init` gains an optional `route: { pathname, params }`. The pathname is held to what a path may be rather than to what a screen may want: rooted, single-slash, no query and no fragment. A protocol-relative `//host` would make `history.replaceState` throw a cross-origin SecurityError and take the mount down with it, and the params are a field of their own so neither side parses a URL. The shell route supplies it, the screen passes it to B4's hook, and the hook holds it for the life of one host: the page routes once, before its first render, so a route that changed afterwards has nothing left to change. The page writes that URL into its history and then mounts. It also hands the same URL to `ExpoRoot` as its `location`, because `ExpoRoot` snapshots `window.location.href` when its module is imported, which is before any frame has crossed the bridge: without it the router reads the `/` the shell served and replaces the page's own path right back. A shell too old to name a route leaves the page with nothing to open, so it paints a panel saying to update the app, built as elements outside React because the route tree is exactly what cannot mount there. Both platforms stop reading the document's URL to decide a load finished. The page rewrites its own path before its first render, so a document that committed at `/` reports finishing at `/h/<hostId>`; reading the path withheld `ready` forever and left the Android WebView hidden behind it. What is left is whether the load committed, which is the question the state machine already answers. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): the desktop lists a page route, the shell honours it or stays native (OTA phase C, C1.3) The worktree list now renders from the desktop's bundle, and which routes do is negotiated rather than decided on one side. The manifest gains `routes: [{ pathname, grants }]`, written from one declared list the builder checks against the tree it bundled, so a declaration naming a screen with no module fails the build instead of reaching a phone as a page that paints Unmatched. The field is additive because the phone reads the manifest loosely and pins no schema version; the desktop's own writer stays `.strict()`, and the stale comment saying there was no additive path is corrected. The shell answers for what it can do. A route the bundle does not list, or lists needing a grant this app does not implement, settles as `native-route` and downloads nothing; so does a desktop that ships no bundle at all, which is the one blocked verdict that is not a wall, because a desktop with no bundle declares no page route and there is no workspace to refuse. The route is answered before the compat verdict for the same reason: a bundle this shell cannot open is not a reason to refuse a screen it was never going to open. `app/h/[hostId]/index.tsx` mounts the shell when the flag is on and takes the native list back as the fallback, and both routes read the flag through one hook so the census stays the whole census. A tap on a worktree row still opens the native session screen. The page posts `notify { name: 'navigate', href }` behind the `navigate` grant, which is not a convention: `notify` is a closed union, so an older shell refuses the whole frame and the page checks the grant before it posts. The shell pushes the target over the still-mounted view, so Back reveals the page with nothing reloaded. `route-handoff.ts` and its web sibling are the seam, router-shaped so the list's own hook and the recorder's adapter are untouched and no golden moves: the web file wraps the three members that leave the document and hands back any target outside the page routes `init` named. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): the page reads this host and keeps the app's pins (OTA phase C, C1.4) Three gaps the census named, and the last of them is why the page needed a second grant. `expo-secure-store` is `{}` on web, so the page's own `loadHosts()` answered with an empty array and the list painted "Host not found" over the host the shell had just opened it for. `init` gains an optional `host`, and `host-store.web.ts` serves it: the profile the screens read, with no device token and no public key, because the bridge already carries the connection those would have opened. The native writes it cannot make — pairing, renaming, recency — settle rather than throw, since recency orders a list the page never shows. AsyncStorage's web build is `window.localStorage`, and the page has none worth having: Android turns DOM storage off and on iOS the origin host is the session id, so a pin set in the page was gone on the next remount. The builder aliases the module to a page store whose values are the app's own — `init` primes the allowlisted keys, a write is applied locally and posted over a new `storage` grant, and the app is where it lands. The allowlist is two keys and is the whole fence: everything the app stores shares one namespace, the hybrid shell flag included, so a page that could write any of it could turn the feature on for a build that never offered it. A key outside the list is refused and, crucially, not kept locally either — a pin that looks set and is not is the failure the grant exists to avoid. The bridge host is built only once both have been read, because `init` is answered once per `ready` and carries them: a host that started without them would have to be torn down to carry them, and the list would already have mounted against a host it could not name. `Alert.alert` on a failed host removal is a silent no-op in React Native Web, so inside the page that failure had no surface at all. It routes to the error the list already shows, on both platforms. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): the page can tell the shell it faulted (OTA phase C, C1.1) A page that throws where it renders has nowhere to report it: the shell sees a document that loaded and a view that never painted, so it waits on a blank page forever. This adds the one frame that says so. `notify { name: 'fault' }` carries the capture an `error` frame already carries, so both directions share one bound and one reader. It rides a grant because `notify` is a closed list on both sides: a page served by a newer desktop into an older shell would have the whole frame refused, so the page asks `init.grants.native` first and stays quiet on a no. The shell answers it as `document-load-failed`, which is what happened. That reason drops the generation and downloads once, so a page broken by bytes this host has since replaced recovers, and one broken by its own code stops at the failure screen rather than a blank one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): give the bridge's notifications and the host's errors their own modules The fault report took both files over the 300-line cap, so each gives up the group that was already separable. The page's one-way members move to `bridge-client-notifications.ts`, which is also where the two policies that split them can be stated: the two the native contract declares throw before a session, and the fault report never throws at all. The host's three error classes move to `bridge-host-errors.ts`, the mirror of the page's own `bridge-client-errors.ts`. No behaviour changes. The commit before this one is over the cap on its own, which a forward-only history is the reason to say rather than hide. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): one boundary under the page's root, reporting to the shell (OTA phase C, C1.1) Nothing in `app/h/**` exports an `ErrorBoundary` and `ExpoRoot` provides no global one, so a throw while a route renders — or a route module that rejects once the manifest is lazy — unmounts the tree and leaves a blank document. The shell sees a load that finished and waits on it forever. The entry now wraps what it mounts on `init` in one boundary that posts the throw over the bridge. Above `ExpoRoot`, not inside its wrapper: a route that cannot be resolved throws where the router renders it, and a boundary below the router never sees that. It renders nothing and offers nothing to press. The generation is on disk and was hash-checked before the view loaded it, so the same bytes throw again and a retry here would only throw twice; recovery belongs to the shell, which drops the generation on the report. The render check now grants the fault and collects what the page posts into the errors every case already asserts empty, because a throw the boundary caught paints nothing and logs nothing a `pageerror` listener would hear. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): write the page-fault callback ref after the commit, not during render React may replay or discard a render, so the write belongs in the commit phase. Layout, not passive, and declared above the host's effect: a native frame can arrive between a commit and a passive effect, and the host must already hold this render's callback. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): write the route ref after the commit, not during render Same class as the page-fault ref: render must stay pure because React can replay or discard it. Folded into the one commit-phase effect above the host's. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): write the page-route and navigate refs after the commit Same class again: the last two writes this branch adds join the commit-phase effect, so nothing this hook holds is written while React is rendering. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): write the storage-write ref after the commit The last render-phase ref write in this hook joins the commit-phase effect. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): name the confirm setter this callback already calls in its deps A `useState` setter is stable, so the identity of the callback is unchanged; the list now says what the body reads. Reported on the line this branch rewrote. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): take the boundary test to C0.5's fake-client pair `createBridgePortPair` is generic over the shell client now; the fake-client form this test wants is `createFakeBridgePortPair`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): bound the wait for a page that never says a word (OTA phase C, C1.1) A route module that throws while the bundle is evaluated takes the entry with it. The document still commits and the WebView still reports it loaded, but no boundary mounts, no fault is posted and no frame is ever sent, so the session sat in `ready` behind a blank view forever. The native view's finished load starts a clock; the page's first `ready` stops it; expiry is `document-load-failed`, which deletes the generation and fetches once. Nothing cancels the timer — a `ready` that lands first makes the expiry a no-op — so the runner owns a clock and the reducer owns every decision. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(config): make a route chunk throw, so the render check proves the boundary reports The check folded page faults into its errors but nothing ever produced one, so a boundary that stopped reporting would have stayed green. The server now serves one real route chunk with a throw in front of it: the module still links, so the failure is an evaluation throw where the router renders, which is exactly what the boundary is for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make the host enforce the grants it issued, and hear nothing before ready `forwardNotify` acted on any frame that parsed, including a `fault` from a page that had never asked for a session and therefore held no grant. Both refusals now go through one rule the host shares with the frame it sends, so the list a page is told about and the list it will be served cannot drift. Inert while every page is offered `fault`; the ungranted arm is what C1.3 needs the moment a grant belongs to a route rather than to the protocol. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): refuse a route no page can open, rather than blanking the WebView (OTA phase C, C1.2) `sendInit` put `options.route` straight on the wire and only the page's decoder checked it, so an out-of-contract pathname made the page refuse the whole `init`, ask again on its 2 s backoff forever, and the shell un-hide a view that would never paint. The only trace was a `console.warn` inside the WebView. Three changes, one failure mode. The host parses the route at construction and serves no session at all when it will not do, reporting it as a shell failure. The pathname rule refuses empty segments, dot segments and backslashes anywhere, because `replaceState` normalises `/../../etc` to `/etc` and `/h/a\b` to `/h/a/b` and the page then renders whatever came out. And the producer encodes the host id it interpolates, which is how one carrying a query, a fragment or whitespace got there. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make a handoff mean the shell took it, not that a frame left (OTA phase C, C1.3) `handOff` returned `client.notifyNavigate(href)`, which answers whether the frame left the page and never whether the shell accepted it. Two hrefs the app builds today were posted, answered true and suppressed the local fallback, so the tap did nothing at all: the Connection-log link's object form, which `String` turns into `[object Object]`, and any href carrying a fragment, because the pathname is stripped to match and the whole href is what goes on the wire. Object hrefs now resolve the way the router resolves them, and the string is checked against the envelope's own pattern and cap before it is posted; anything that fails falls through to the local router, which is the policy this module already states. Whether a target names a screen that exists is shape's business no longer, and the comment says C1.7 owns it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep a failed action off the whole screen and the page's store honest (OTA phase C, C1.4) Five, from one round of review. A removal that fails no longer writes the identity error: that one is an early return over the header, the list and the overlays, with nothing to dismiss it and nothing left to render the confirm it re-opens. It goes to a dismissible line above the list instead, on both platforms, cleared by the next confirmed refresh. `init` reads the allowlisted keys on every answer rather than capturing them at mount, so a document that reloads inside one mount is primed from after its own writes. The read stays synchronous: the page refuses every member until `init` lands and the golden recorder mounts a screen in the same turn it drains one, so a promise here moves the first render of every bridged replay. A profile read that rejects is now a shell failure with a diagnostic instead of a `ready` session with no host behind it and a page asking forever. The page bounds a value by the envelope's own constant rather than caching what the wire drops. And a write is held to the keys this page was handed, so one host's page cannot rewrite another's pinned list. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): state the two test fixtures' types instead of asserting them The casting gate reads a `SAFETY:` rationale off the line directly above the assertion, and a wrapped comment puts a comment there instead. Two of the four were not assertions worth keeping at all: a hoisted fixture says its own type, and the router comes from the mock the file already installs. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert the two fixtures in place, not through a widened binding `const x: unknown = …` followed by `x as T` is the widen-then-assert the anti-slop gate refuses, and rightly: the evidence is discarded and then invented again. The assertion belongs at the literal, with its rationale on the line above it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): start a new flow when the shell view remounts A remount cleared `pageReady` but left the flow alone, so the wait the retired document armed still matched. It expired onto the page that replaced it, took a ready workspace to `document-load-failed`, and deleted the generation on the way. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): say which page notification the bridge refused and why A refused `notify` fell through to the line about a view outliving its host, which is a different fault and names neither the notification nor the reason. The two refusals now get a line each, so a page that was told nothing cannot bury one reaching past what it was told. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the ready deadline to the page's own retry ceiling The margin was stated in a comment and asserted against itself, so changing either number left the suite green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): say what was wrong with the screen a refused shell named C1.1's per-kind log lands on a branch that also refuses a route, and that diagnostic was still falling through to the line about a view outliving its host. It names the shell's own bug now, and carries the issue. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): refuse a dot segment however the route spells it A URL parser percent-decodes a path before it resolves it, so `/h/%2e%2e/x` climbed out of the `/h/` prefix exactly as `/h/../x` does and landed the page on a screen nobody asked for, with no refusal anywhere. The one segment rule both patterns share now reads the encoded spellings as the dot segments they are, and still lets an escape inside a name through. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): name routes in the manifest field list the builder emits C1.3 added `routes` to every manifest this builder writes, and the Phase A contract test still listed eight keys, which is what went red in CI. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): hold a navigate target to the same segment rule as the shell's The href pattern is built from the segment source C1.2 tightened, and nothing said so: a spelling one pattern refused while the other took it would be a hole with a `notify` already pointed at it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): name the key a refused page write reached for The last diagnostic still falling through to the line about a view outliving its host, on the branch that added it. The key is the evidence: it says which host's pinned list the page was reaching into. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): give the ref-refresh probe the navigations this branch added C1.1's new case builds its own probe, and on this branch a probe also collects the hrefs the page hands back. The file stopped typechecking on the merge, which the tests ratchet caught. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): give the ref-refresh probe this branch's storage writes too Same merge, one branch further: a probe here also collects what the page asked the screen to write. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile): format the web shell route entry Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): hand the page the app's storage as it stands, not one init late The page is handed its allowlisted keys on every `init`, built synchronously, and the app writes two of them from its own native screens. The shell's map was only re-read after a ready, so a native write between two readies reached the init after next: the drawer opened on the repo the user left. The map is now module-scoped and every writer of an allowlisted key notes it as it writes, so the init that answers a ready already carries it. The store read only seats the map, and a read that started before a write no longer puts the older value back. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): find the banner's dismiss without an assertion Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): keep the storage mirror in the storage tree The app's own writers had to reach into `src/mobile-web-shell/` to note a write, which is the dependency the wrong way round: the shell is what is built on the app's storage, not the other way. The mirror moves to `src/storage/` and no longer knows which keys the page is allowed; the caller names them on every read and every seat, so the allowlist stays where it is enforced. No behaviour change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): split the bridge frame suite along the modules the merge created `bridge-rpc-client-frames.test.ts` reached 835 counted lines once C0.8 and C1.1 both added cases to it, over the 800 the lint allows. The split follows the two modules those changes extracted, so each suite now names the module it covers. `bridge client page faults` moves to `bridge-client-notifications.test.ts` (the outbound notify surface) and `bridge client refusals and send failures` to `bridge-client-inbound-frames.test.ts` (the reader, including the refused-event release that cancels at the shell). The seven suites that exercise the client as a whole stay put. The fake port all three drive moves to `bridge-page-client-test-harness.ts` rather than being copied three times. No case changed and none was dropped: 48 `it` cases before, 37 + 4 + 7 after, and all nine `describe` bodies compare byte-identical to their originals. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): type the shared init fixture as the member a case reads The harness exported `INIT` as `BridgeHostMessage`. While it was a module-local const, control flow narrowed it to the `init` member at each use, so `INIT.grants` read fine. An imported binding keeps its declared type instead, so the same read lost `grants` to the union and the tests ratchet went red. Declared as the init member, which is what every case already treats it as. No cast: the object literal is checked against the narrower type directly. `INIT` was the only exported fixture with this shape. `CONNECTION` is `as const`, `GRANTS` is inferred, and nothing reads a member off an `eventFrame` result. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 22:20:49 -04:00
Jinwoo Hong	1e7a69710d	feat(mobile): update wall for the desktop-served mobile web bundle (OTA phase B, 1/4) (#21411) ... * feat(mobile): decide whether a web bundle may open against its host A pure verdict for the bundle update wall, ordered so the answer names the soonest cause: a host with no bundle has no manifest to disagree about, and an unknown manifest schema makes the protocol window inside it unreadable. Same `?? 0` defaults as `evaluateCompat`, so an absent status field reads as the oldest host that could have answered rather than as permission. Every blocked verdict is terminal. There is no native workspace fallback, so each one carries the numbers it compared for the support breadcrumb. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read an unknown bundle schemaVersion through to the wall The client reader pinned `schemaVersion` to the one schema this shell knows, so a future schema 2 failed the parse before `evaluateMobileWebBundleCompat` could call the shell too old. The user would have seen a transport error where the update wall belongs. The host's own manifest stays closed in both directions, where it is written. Goldens are unaffected: every recorded reply carries schema 1. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): give the block screen copy for the bundle walls One component still renders every wall. `updateSide` picks the app to update from the reason, so the copy and the store link cannot disagree, and a new reason is a compile error there rather than a mobile title over a desktop button. The existing protocol copy is unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): type the bundle protocol window the update wall compares The loose reader left `runtimeProtocolVersion` and `minCompatibleRuntimeProtocolVersion` as unknown index members, so the parsed manifest could not reach `evaluateMobileWebBundleCompat` without a cast. Both are now read as non-negative ints, and the reply-schema test pins it at the call site: the wall is invoked on a parsed manifest, so dropping either field stops compiling. A host that omits the window is now refused. Only a host too old to advertise `mobileWeb.bundle.v1` can send one, and the phone never asks such a host for a manifest. The probe test's fake manifest gained the fields it was missing, which is the typed reader catching its first stale fixture. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): say whether a bundle verdict actually checked a manifest `ok` meant two different things: the manifest was read and its window contains the host, or no manifest had been read at all. A caller that mounted on the second would mount an unchecked bundle, so `manifestChecked` separates permission to fetch from permission to open. The host-status input is now a `Pick` of `HostStatusReply` instead of a hand-copied pair. Both fields default through `?? 0`, so an upstream rename would have silently blocked every host rather than failing a build. Drops two assertions that restated the module's own literal back at it. What proves today's bundle opens is that the shared contract's schema version is a member of the supported list, so that is the assertion left standing. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): offer a refetch, not a store, for a bundle the host outgrew `bundle-incompatible` on the mobile side means the workspace cached for this host is older than the host's client floor. A store update cannot clear that and a reconnect can, so the screen no longer sends the user to a download that would change nothing. The button is gone rather than relabelled, because the recovery is leaving this screen, and the note drops its "already updated?" opener for the same reason. `blockRemedy` replaces `updateSide` and is now passed to the copy instead of recomputed there, so the title, the body, and the button are decided once. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): drop the platform assertion from the block-screen mock The mocked `Platform.OS` was widened with an assertion so a test could switch stores. An annotation on the binding does the same widening in a position the compiler checks, which is what the changed-code quality gate asks for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say which bundle-compat default is fail-open, and drop a dead field Both comments claimed the two host-status defaults point the same way. Only `protocolVersion` is absent-means-oldest. An absent `minCompatibleMobileVersion` is `?? 0`, which is no floor at all, so the mobile arm is fail-open by design and matches `evaluateCompat`. A reader taking the old sentence at face value would have gone looking for a bug. `supportedSchemaVersions` had no consumer on the verdict: the block screen renders a title and body, and B4 reads neither. The exported constant stays, since that is what the wall is decided against. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 04:27:45 -04:00
Jinwoo Hong	40b2230508	test(mobile): typecheck the test files on a ratchet, and pin the reply enums where tsc looks (#21298) ... * fix(mobile): move the last six reply-enum pins where tsc looks mobile/tsconfig.json excludes *.test.ts, so a `Record<HostUnion, true>` coverage record in a schema test is never typechecked: the two that existed (SshConnectionStatus, GitHubProjectOwnerType) checked nothing, and the four closed enums beside them had only a doc citation of the host type. Each arm list moves into its schema module as hostUnionArms<Union>(), which #21269 introduced for the same reason, and each test iterates the exported list instead of holding its own copy: - SSH_CONNECTION_STATUS to SshConnectionStatus - PROJECT_OWNER_TYPE to GitHubProjectOwnerType - DETAIL_FILE_STATUS to GitHubPRFile['status'] - PUSH_TEST_REFUSAL_REASONS and PUSH_REGISTER_REFUSAL_REASONS to the refusal arms of MobilePushTestResult and MobilePushRegisterResult - SETUP_RUN_POLICIES to SetupRunPolicy openEnum's parameter widens from a non-empty tuple to `readonly string[]` so a hostUnionArms list can feed it. z.enum already accepts the same, so the tuple constraint only excluded callers zod itself takes; behaviour unchanged. Twelve mutations prove the pins: dropping one arm and adding a bogus one each fail mobile tsc in all six places. Zero goldens move, the schemas' behaviour being unchanged, and the 21 recording suites pass at the existing baseline. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): fix the type errors in eighteen test files Found by typechecking the tests for the first time (see the config that follows). All mechanical, none weakens a product type: - 67 `act(() => vi.advanceTimersByTime(...))` callbacks return VitestUtils where act wants void, so each becomes a block. The async ones await only a genuinely promise-returning call, so no extra microtask tick is introduced. - Four fixtures were stale against a product type that gained a required member: MobileViewState.alwaysShowDefaultBranch, PrSidebarData.checksError, the branch-compare summary's errorMessage, and SessionOptionDescriptor's transport, which #20884 added precisely so a producer could not inherit the wrong lane's rendering by omission. - `getLastConnectedAt` on the shared relay fake was typed `() => null`, which refused the timestamp two escalation suites assign to it. - Two holders used before assignment take `!`, one `advance!.kind === ...` becomes `advance?.kind`, one widened status arm takes `as const`, and the Expo notification fixture keeps `data` required because the dismissal cases assign through it. 631 test files pass, 6222 tests, unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): typecheck the test files, on a ratchet mobile/tsconfig.json excludes *.test.ts so Metro never compiles tests into the release bundle, and vitest transpiles without checking types. Nothing had ever typechecked a mobile test, which is why a `Record<HostUnion, true>` pin written in one proved nothing and why 144 of the 630 test files had drifted. tsconfig.test.json is that program with the tests put back, behind `typecheck:tests`. Four files stay out: they import the desktop main process or src/shared/child-process, which are written against @types/node, and this program's libs are React Native's, where setTimeout answers a number rather than a NodeJS.Timeout. Pulling that graph in reports ~280 errors about the desktop rather than about mobile; vitest runs those four under Node, which is where they belong. The CI gate is a ratchet rather than the raw typecheck, modelled on check-ts-nocheck-ratchet.mjs: 126 files still fail, so the gate freezes that set and fails when a file that checks today stops checking, or when a baseline entry starts checking and was not pruned. The list may only shrink. Why not zero: 180 of the remaining 510 errors are one seam — tests locate mocked react-native components by string name, which `ElementType` does not admit — and closing it means either 180 casts or a global JSX declaration for the mocked names. That is a design decision, not a mechanical fix, so it is left for a follow-up rather than made here. The rest are smaller clusters of the same kind: vi.fn mocks assigned into typed slots, call-arg tuple indexing, and createElement props fixtures. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile-recorder): correct the corpus counts and the salvage claim The oracle section still quoted the corpus as 368 scenarios and 727 goldens; it is 393 and 778, and the three replay suites report 781 tests. Each number now names the command that measures it. "No golden carries one" was the load-bearing error: 44 goldens carry a recorded `reply-salvage` today, starting with the push-test unknown-reason scenario #21176 added for exactly that purpose. The paragraph claimed the observation pins an absence when on those families it pins a recorded drop. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the tests-typecheck ratchet's parser The gate reads tsc's output, and tsc indents the "Overload 1 of 2, ..." detail under an error. Counting those as filenames would write unparseable entries into the baseline and leave the gate unprunable, so the parser is pinned on that shape as well as on the added/stale diff. Written against the gate itself: it flagged this file before the directive it carried was removed, which is the end-to-end proof the spawn half works. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): await the timer advances the act() rewrite dropped Rewriting `await act(async () => vi.advanceTimersByTimeAsync(n))` into a braced body left the returned promise floating at 27 sites, so the advance was no longer ordered before the assertions that follow it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): unshadow MobileHostCard's .tsx suite A wildcard `include` keeps only the higher-priority extension, so MobileHostCard.test.tsx sat outside every tsc program while MobileHostCard.test.ts existed beside it. Its one error is the same react-test-renderer seam its sibling is baselined for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): census every test file into the typecheck program The ratchet diffs only files that error, so a test excluded from tsconfig.test.json or shadowed by a sibling extension left the gate silently. Every *.test.ts(x) on disk must now be in the program or named in TESTS_OUTSIDE_PROGRAM with its reason. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(shared): make the enum helpers refuse the ways they can prove nothing openEnum takes a `const` T so a bare literal keeps its arms rather than widening to string. hostUnionArms blocks inference of U with NoInfer and defaults it to never, so a call that omits the host union — where the record would only pin itself — no longer compiles. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): describe the census and correct the baseline count Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): give the push fixture cast its SAFETY rationale Widening the pre-existing cast made the changed-code gate attribute it as a new finding. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): build the push fixtures as typed notifications Replaces the `as unknown as` cast with Expo's own types, filling FirebaseRemoteMessage and its notification once in two builders, and passes the data payload in rather than mutating through an optional member. Typing the fixture showed one assertion comparing the scheduled content against the whole arriving content, which only held while the cast let the fixture omit the two members the presenter drops; it now names the four members the presenter forwards. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): keep the grouped-question advance read non-optional `advance?.kind` let an absent advance take the null-draft branch instead of failing. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): run the tests-typecheck ratchet on Windows Spawns tsc's JS entry on this Node instead of the node_modules/.bin shim, which is a POSIX shell script that Windows resolves to tsc.CMD and then appends .exe to. Parsed paths are normalised to POSIX so a Windows run does not read every baseline entry as both stale and added. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): close the ratchet's @ts-nocheck hole and read tsc once tsc exits 0 on a @ts-nocheck file, so a baselined test could be "fixed" with one line, pruned, and never checked again; the census now names any program test file whose leading comment carries the directive. `--noEmit --listFiles` answers both questions in one pass, so the gate spawns tsc once rather than twice. Corrects the two stale counts, and states hostUnionArms' real reason for living in the schema module now that tests are typechecked. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-17 18:58:25 -04:00
Jinwoo Hong	4a86b2dc56	refactor(mobile): checked reply readers for files, dictation, host-screen and agent-history (step 7) (#21269) ... * test(mobile): record main's file-preview and markdown-disk-fallback replies Four of this branch's read sites had no malformed-reply coverage, so the reader change would have had nothing to move at them. `familyGoldens` matrixes only the first scenario of each family, and `files.preview-load`'s base is the grant-refresh chain while `session.tab-documents`' is the served markdown tab — which left `files.read` and `files.readPreview` on the worktree preview path, the artifact image read, and the markdown tab's on-disk fallback recorded on their success path only. This commit is the before picture, taken from main's own tree with no product edit in it. Three new families, five scenarios, ten goldens: - `files.preview-worktree-text` / `files.preview-worktree-image` — `files.read` and `files.readPreview` as the preview screen asks them for a worktree file. - `files.preview-artifact-image` — `files.readTerminalArtifactPreview`. - `session.markdown-disk-fallback` — the `files.read` leg a headless host's `renderer_unavailable` sends the markdown tab down. It carries a second scenario that serves `markdown.readTab`, because a matrix site needs a fulfilled reply recorded somewhere in its own family to replay as the `normal` partition. No existing scenario moved to a new family and no adapter changed, so every pre-existing golden keeps its `adapterSha256` and `scenarioSha256`. Recorded in a detached worktree at the manifest's pin (`4b876758d3`) with this manifest copied in; the control is that all 748 pre-existing goldens came back byte-identical to origin/main's, which `git diff c2962a765a -- mobile/rpc-foundation/goldens` confirms as empty. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): checked reply readers for files, dictation, host-screen and agent-history Thirty-five unchecked reply readers across seven files become checked zod readers, so a malformed host reply surfaces as one readable error at the operation boundary instead of a downstream TypeError, a rendered `undefined`, or a screen left ready over garbage. Deliberately a behaviour change on malformed replies only: every good reply decodes to the same value it did, which the `normal` partition of every matrix golden holds byte for byte. Nothing on the wire moves — no method, params, options, timeout or acceptance policy changes at any site. The inventory drops from 137 readers over 31 files to 102 over 24. What each domain checks, and what it deliberately does not: - files/preview — one schema for `files.read` and `files.readTerminalArtifact`, one for the two preview methods. `content` is required on the text pair because the markdown disk fallback publishes it into the tab with no guard; the image pair requires nothing, because normalizeImagePreviewResult guards all four members and the host's own "binary I cannot preview" and "not actually an image" arms are good replies the screen renders today. - files/tab-doc — stricter than the preview screen on the same two methods, because a tab publishes what it read into a typed ready document with no guard. `git.diff` reads as two variants, and an arm this build has not heard of takes the binary one rather than refusing the reply. - files/explorer — the directory listing is an array and a row needs the name and the directory flag the tree projection turns on; the legacy capped list needs its rows' paths and the truncation flag its note draws. - files/ownership — the two members that decide *where a write lands* are fatal on a wrong type rather than salvaged, because absence reads as `local` downstream and a salvage would send a mutation to the wrong host. `hostId`'s absent/null/string states stay distinct, and the SSH connection generation passes through at its own type because the mutation echoes it back to the host. - dictation — the setup the sheet renders is checked; the model rows need the `id` the sheet keys and sends back. The five sends whose reply body no call site reads keep an unknown payload, and so does `speech.dictation.finish`, whose transcript is read past a staleness guard that a reader throw would move the failure across. - host-screen — the repo catalog, the SSH labels and the host platform. The four writes read no reply body; `worktree.activate` stays opaque because the session route's second report site awaits it outside any catch. - agent-history — the capability gate and both scan containers. The session rows stay unknown on purpose: `agent` is a vocabulary that grows with every agent CLI Orca learns to scan and that this client echoes back on resume, so narrowing it would refuse a newer host's reply or drop the very sessions it added. Two shared readers were widened to take the strings the reply readers hand them — `getRepoExecutionHostId` and `buildRepoHostIdByRepoId` — because both already answer `local` for a host-id spelling they cannot parse, and closing that spelling in a reply schema would refuse a newer host's own catalog. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus and re-record the checked reply readers `baseline` moves to this branch's last fenced commit, which is what `--record` refuses without: main's fenced tree drifted past the session domain's pin when #21114 and the dependency bump landed, and the product edit in the commit before this one moves it again. Every body move is confined to a malformed partition of a family this branch touched. No `normal` partition moved, which is the byte-for-byte control on good replies, and no golden outside the seven files' families moved at all. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): stop the dictation reader requiring a mode main rendered without The setup sheet's `normal` partition refused after the reader landed, which is the success control saying the schema was wrong rather than the fixture: `dictationMode` was declared required because the one unguarded consumer pushes it into a `useState<'toggle' | 'hold'>` and cannot invent a value, but main rendered a sheet whose reply omitted it, and requiring a member no consumer crashes on is exactly the version claim Rule 1 of the remote-wire contract warns about. The member is salvaged now and keeps its open arm set, so an unknown mode still degrades to `toggle` rather than to one that matches no segment. The native-chat refresh spells that same `toggle` for an absent mode, which is the value its state already started at, and the route parity pins are refreshed for the one literal and the two callback bodies that moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin past the dictation fix and re-record Second repin of the branch: the fix to the setup reader is a fenced-tree change, so `--record` refuses until `baseline` names it. The speech family's `normal` partition is back to main's projection, which is what said the first reader was wrong. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): mutant evidence for the checked reply readers Three mutations applied by hand, run, and reverted, recorded beside the adapter family mutations in the same shape. They are kept in their own file because a reader mutation is not killed by a pilot scenario: a pilot serves a good reply, and a schema that has stopped checking a member reads a good reply exactly as before. What kills them is a matrix golden's malformed partition, the schema's unit pin, or a consumer pin, and each is named against its mutation. Two survived their first run, and both survivals were defects in the gates: - Loosening the file tab's `content` was invisible, because the pin dropped members only in pairs and each pair is refused by the sibling. The pin now drops exactly one member per iteration, and the preview text schema and the legacy file list got the same treatment. - Collapsing the hostId tri-state was invisible, because no golden serves an explicit null host — the local ownership scenario omits the member. The ownership test now captures all three states end to end, which is where a tri-state belongs. `repo-metadata-platform` is re-anchored where this branch moved the read it mutates: the hand-rolled `readHostPlatform` became the reply schema's own projection. The defect it injects is unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record main's repo-icon and speech-vocabulary replies The closed enums this branch introduced had no fixture behind them. `provider`, `dictationMode` and `repoIcon` were carried by no scenario at all — the fulfilled repo-metadata golden records `repoIconsByName: []` — so the corpus could not have moved whatever arm set the schemas declared, which is how a reader can pin a vocabulary the host does not speak and still decode to a zero-move delta. Two scenarios, both appended to an existing family so `familyGoldens` adds no matrix golden, recorded from main's own tree at the pin with no product edit in it: - `settings-repo-metadata-icons` — all three `RepoIcon` arms, a github-sourced image with a label, an explicit `badgeColor`, and a mixed-host catalog so the ssh/settings/platform wave runs too. - `speech-setup-sheet-model-vocabulary` — `provider` on both arms, `status` on two, `dictationMode: "hold"`, and null and numeric `sizeBytes`/`progress`. Control: re-recording the whole corpus at the pin reproduces every committed golden body, including this branch's five earlier before-pictures; only `baseline` and the masked `lockfileSha256` move. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): stop the repo icon narrowing a member no consumer reads The image arm of `repoIcon` declared `source` as the four values `RepoIconImageSource` spells today (src/shared/repo-icon.ts:3). MobileRepoIcon reads `type`, `src`, `label`, `emoji` and `name`, and never `source`, so the only thing that enum could do was fail the union arm for a source a later host adds — dropping the whole icon and drawing the Folder default where main drew the image. That is the one arm set on this branch whose degrade was not already main's own behaviour for an unknown value. Dropping the declaration keeps the member: `looseObject` passes it through verbatim, so the decoded object is byte-identical to the one main published, which `settings-repo-metadata-icons` now records. The two type sites that hold an icon move to the decoded type. A host `RepoIcon` still satisfies the rendered union, so the worktree rows that carry one are unaffected. Every other closed enum on this branch was checked against the host's own shared type and left alone: speech `provider`/`status`/`dictationMode` (runtime-worktree-contracts.ts:83/85/86), `groupBy`/`sortBy` (persisted-ui-state-types.ts:41-42), `platform` (Node's own domain; the handler answers `process.platform`). For each, a salvaged member lands on the same branch main's unknown value did: `=== 'openai'` and `=== 'ready'` stay false, a missing `groupBy` and an unmapped one both answer null, and an unknown platform and a null one both label the host "This computer". Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin past the repo-icon fix and re-record Header-only: all 770 goldens move on `baseline` alone, including the two recorded from main's tree two commits back. The icon fix and the two new fixtures decode to the bytes main published. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the repo-metadata readers total the way main's were readSshTargets and readHostPlatform answered [] and null for any payload at all. The checked schemas threw for a non-object, and because the label write runs first in the same sequence that throw also skipped the platform write, so a malformed reply left both decorative labels at their previous values instead of degrading. A .catch on each restores main's answer without giving up the row filter or the checked reader. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): forward the dictation mode instead of substituting a default The reader closed the mode to two arms and the native-chat refresh spelled `?? 'toggle'`, which is a good-reply change no golden covers: main left the state undefined for a reply that omits the mode, and undefined binds no press handler on the terminal input mic. Head gave that mic a working toggle. The member is forwarded as the string the host sent and the refresh is main's line again, so an absent or unknown mode leaves the mic exactly as inert as main's. The route-parity runtime-string pin is main's own sha again. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin past the review fixes and re-record The repo-metadata readers are total again, so both families' `result-absent` and `result-null` checkpoints decode to main's bytes instead of the caught throw, and the two delta rows they cost go away. The dictation mode forwards verbatim, which no recorded reply exercises differently. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin onto the merge and re-record Pins the corpus to the merge commit so main's ten create-terminal goldens and this branch's own are recorded from one tree. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct three reader comments round 2 caught The ownership schema said an explicit null hostId means the host said local; the code refuses it, which is the whole reason mutant (c) exists. The AiVault sessions cast cited a golden whose fixture row carries three members, not the sixteen the cast claims — the full row is in aivault-history-screen-listed — and both the issues cast and the schema doc said the rows are rendered when the only read anywhere is issues.length. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct stale file:line citations in the batch-A reply schemas Resolved every citation in the seven reply-schema modules and the SAFETY notes against the tree and diffed each target line against the claim beside it. Twelve were wrong, two of them past the end of a file that had shrunk, so they read as evidence while pointing at a closing brace. - file-explorer: the entries put is :157 not :160, the relativePath split is file-list-fallback.ts:48 not :42, and the truncated publish is :136 not :141. buildFileExplorerRows is no symbol at all; the sort-and-walk is flattenDirectoryCache (file-tree.ts:58). - file-ownership: the !summary throw is :68 not :64. - file-preview: the markdown disk fallback reads content at :60 not :65. - file-tab-doc: the html body render is :68 not :81 and the file arm is :73-75 not :86-88 (the file has 78 lines); the isImage guard is :58 not :66; the kind !== 'text' branch is :41 not :44; mobileDiffImageDataUri spans :22-33 not :20-31; the unguarded content.length is mobile-diff-lines.ts:35, the function that does it rather than :34. - agent-history: both members land at :133-135; :135 alone is issues. - dictation: the parenthetical read as citing the staleness guard when it named the rpcPayloadMember read. Both are cited now, :237 and :225. Comments only. No schema, type, or runtime behaviour changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): name the unguarded activation report site that pins the opaque schema Handler audit over all 33 interpret sites in the four domains found one site that is structurally unguarded: use-mobile-session-startup.ts:170 reports the activation verdict from inside a fire-and-forget `void (async …)()` whose only `.catch` sits on the request, not on the chain. A throw there would be an unhandled rejection and would also skip the terminal fetch below it. Nothing throws there today, because `worktree.activate` reads hostScreenUnreadReplySchema, which is `z.unknown()`. That totality is load bearing rather than incidental, so the doc now names the line it protects and contrasts it with the first report site at :141, which is chained `.then(…).catch(…)` and would survive a throw. Comments only. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin that a bound descriptor's interpret survives being detached bindDeferredRpcOperation builds interpret as a shorthand method closing over the captured operation, never `this`, which is what lets eleven call sites pass it as a bare function reference. Nothing named that invariant. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): repin the RPC recording baseline to the main merge Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): pin the closed reply enums to the host unions where tsc looks pullfrog: the PR body promised a Record<HostUnion, true> pin for every closed enum in this batch and the code had none. Adding them in the schema tests would have changed nothing: mobile/tsconfig.json excludes *.test.ts, so a coverage record there is never typechecked (a mutation that dropped a key stayed green). hostUnionArms(coverage) in zod-salvage spells the arm list as a Readonly<Record<U, true>> in the schema module itself, called with the host union as the explicit type argument: an arm the host adds is a missing property, one it drops is an excess property. Used for the speech provider and status (RuntimeSpeechModelSummary), the workspace groupBy and sortBy (PersistedUIState) and Node's platform list, which host-screen now imports from mobile-runtime-host-platform instead of duplicating. The repo icon branches satisfy Readonly<Record<RepoIcon['type'], z.ZodType>>. Three mutations (drop `manual`, add `bogus`, drop the image branch) each fail tsc. The tests iterate the exported lists; the platform mutant is re-anchored to the renamed constant. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-17 16:38:16 -04:00
Jinwoo Hong	6142657d7a	refactor(mobile): checked reply readers for the tasks domain's board, runtime, search and create (step 7) (#21246) ... * test(mobile): record main's agent.launch create receipt before checking it `agent.launch` is the one read site in the tasks domain's project-board, runtime, source-search and workspace create/source files with no recording family at all, so main's answer to a malformed launch receipt was undocumented and a checked reader would have had nothing to move. One family, one scenario, two goldens: `worktree.agent-launch-create` drives `createWorktreeWithNameRetry` down the `agent.launch` arm instead of `worktree.create`, which needs an `agentLaunch` argument on the existing worktree-create-retry adapter. The agent is a constant there on purpose — which agent is picked changes only the params, and the arm under test is which method the create is issued on. A separate family rather than an eighth `worktree.create-retry` scenario: `familyGoldens` drives its reply matrix over the family's FIRST scenario, so adding to that family would have recorded a pilot golden and left the launch receipt with no partitions. As its own base it gets all eleven. Recorded from a detached worktree at the pinned baseline with this branch's `rpc-recording/` and manifest copied in, per the recipe in the recorder README: `mobile/pnpm-lock.yaml` has drifted past `4b876758d3` on main, so `--record` refuses on this branch's tree even though `mobile/src` and `src/shared` are byte-identical to the pin. Thirty-four existing goldens move on `adapterSha256` and nothing else — the six families mounted through the edited adapter module. No body moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): checked reply readers for the tasks domain's board, runtime, search and create Forty-three unchecked reply readers across five files become checked zod readers, so a malformed host reply surfaces as one readable error naming the method instead of a downstream TypeError, a rendered `undefined`, or a screen left ready over garbage. Deliberately a behaviour change on malformed replies only. Five schema modules, each recording the consumer line behind every requirement and the host handler it was checked against: - `task-project-board-reply-schema.ts` — the sixteen `github.project.*` envelopes. Where a consumer reads a member off BOTH arms unguarded the schema is a union on `ok`; where it guards everything (`result.error?.message ?? '…'`, `result.labels ?? []`) it is a flat passthrough and requires only the container, because a requirement on a member the consumer already defaults would refuse a reply main rendered. - `task-runtime-reply-schema.ts` — the hydration reads. The three preference writes read `z.unknown()`: no call site interprets their body. - `task-source-search-reply-schema.ts` — the provider searches and the pasted single-item lookups. The Linear union replaces the hand reader in linear-mobile-issue-read.ts, whose own copy reached the screen unattributed. - `workspace-source-reply-schema.ts` — SSH state, agent detection, orca.yaml hooks, sparse presets and base-ref search. - `workspace-create-reply-schema.ts` — the create receipt, the launch receipt and the hosted-base union. Requirements are exactly the members a consumer reads unguarded AND a recorded golden shows the host sending. That second half is load-bearing: the recorded GitHub search row is `{ number, title }`, the recorded Linear issue is `{ id }`, the recorded project is missing `id`/`url`/`source` and the recorded sparse preset is missing `repoId`/`createdAt`/`updatedAt` — requiring what the shared types declare would have dropped rows main renders. Where the value therefore stays looser than the screen's own state type, the call site keeps one narrowing cast with that reason on it rather than a default that would fabricate state. Two enum decisions, both pinned: - `ownerType` is CLOSED with no fallback. It is echoed into the next `github.project.listViews` params, and remote-wire-compatibility.md rule 4 forbids a reply-schema fallback from shaping a param; the host's own listing handler answers `validation_error` for any other value. - `ssh` `status` is OPEN and degrades to `disconnected`, main's own answer for a state it did not receive. The readiness gate is an equality test against `connected`, so an arm this build has not heard of can never grant a create, and the record survives with its Connect affordance. - Every other host vocabulary a consumer equality-tests — the project view `layout`, the `setupRunPolicy` — stays `z.string()` for the same rule. Tri-states are preserved, not collapsed: the row detail's `reviewDecision`, a work item's `author` and the SSH record's `error` each keep explicit `null` distinct from absent, with a unit pin on each. `blank-workspace-create.test.ts` splits one `it.each` in two. The two create routes now answer a workspace-less reply differently: `agent.launch` still reports "Failed to create workspace", because its reader guards `worktreeId` itself, while `worktree.create` is named as unreadable, because the create screen reads `result.worktree.id` unguarded into the session route. Both reach the same catch; only the sentence changes. `mobile-tasks-refactor-parity.test.ts` moves four hashes and no count. Hooks hold at 350 with 28 bodies edited and no dependency array moved; statements hold at 417 and declarations at 194; `semantics` loses exactly four lines, all four string literals that lived inside the one deleted inline cast type. No method literal and no `rpc:` call signature moves. The inventory loses its five tasks lines; the boundary test stays green. Goldens are refreshed in the next commit, which is where the disclosed behaviour change is proved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record the corpus over the tasks domain's checked readers Repins `baseline` to d4cfac98b4, the commit that landed the checked readers, and re-records all 760 goldens. The repin rewrites that header on every file; the recorder edit below rewrites `recorderSha256` on every file too. The disclosed behaviour change is the body-moved set and nothing else: the malformed reply partitions of the families whose readers this branch converted. Two recorder files move with it, both re-anchoring evidence the checked readers displaced rather than deleted: - `pilot-recordings.test.ts` restates the b2 seed. The shipped null result is still the seed and the screen still reports an error the user can see; what moved is the sentence, from V8's "Cannot read properties of null (reading 'ok')" to the reply and method the reader names. - `operation-mutations.ts` re-anchors that seed's `acceptance` mutant. Its defect is a null envelope reaching the metadata sheet, and the call-site guard it was injected at can no longer see one, because the reader refuses the envelope first. The anchor is the schema now, and loosening it to `z.unknown()` puts the null back on the path to `result.ok` — the same defect at its new home. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert the reply-schema pins without type assertions The changed-code casting gate counts a `as` in a test like any other, and eight of them had crept into the new schema pins. Each is replaced by an assertion that reads the same fact off the typed value: the schema already declares `worktreeCreateIdempotency`, `glab`, `status` and `error`, so the narrowing was never needed, and the two "is this key present" checks are JSON comparisons, which is the honest way to ask — `JSON.stringify` drops an absent key and keeps an explicit null, which is the whole distinction a tri-state pin is making. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the tree it was recorded from The previous repin named d4cfac98b4, and the assertion cleanup that followed it touched `mobile/src` — a fenced path — so the header pointed at a tree the working copy no longer was. Repins to 6b740c3f61 and re-records. Bodies are unchanged: only `baseline` moves, on all 760 goldens. Four test files cannot reach a recording, which is the point — the fence does not know that, and a header that names a tree nobody can reproduce is the one claim it exists to make. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): point the reply schemas' consumer citations at the landed lines Every requirement in the five schema modules names the consumer line that justifies it, and the migration moved those lines: deleting a thirty-line inline cast type shifts everything under it. The citations now resolve against the tree they ship in. Comment-only. No schema, no reader and no consumer changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the tree the citations landed in `baseline` follows the last commit to touch a fenced path, and the citation fix did. Bodies unchanged: `baseline` moves on all 760 goldens and nothing else. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin both reader vocabularies against the host's own unions The two enums these readers declare were checked against mobile's restatement of the wire, not against the types the handlers return. A closed enum written from the wrong vocabulary drops every row that carries an arm it omits, and no golden can catch it when no fixture carries one. Both arm sets are now keyed by the host type in a Record, so an arm added to or removed from SshConnectionStatus or GitHubProjectOwnerType fails tsc before any test runs. The SSH degrade's inertness is pinned at the gate that reads it rather than argued in a comment: an arm a newer host sends and the degraded value reach the same label, the same readiness verdict and the same error. Also corrects a comment claiming the file-mutation owner check reads members this schema forwards. It asks ssh.getState through a reader of its own, and no mobile code reads providerEpoch, supportsFolderDownload or remotePlatform. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the tree the vocabulary pins landed in Comments and tests cannot change a decoded value, so the whole delta is the baseline header key: 760 goldens, one line each, no body moves and no scenario or adapter change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record real provider rows in the smart-search and paste fixtures The smart-search and paste-lookup scenarios carried hand-written stubs, not rows any handler can build: Linear issues of `{ id }` alone, GitHub items of `{ number, title }`, and a GitLab item keyed by `iid`, a member neither work-item type declares. Every one of them omits members the host's own types declare non-optional and mobile then reads with no guard, so the corpus was evidence for a requirement it could never have justified. The rows are now the shapes the corpus already uses elsewhere (`tk-list-linear`, `tk-provider-load`, `tk-list-gitlab-items`), checked member for member against LinearIssue (src/shared/linear/issue-types.ts:3), GitHubWorkItem (src/shared/github/work-item-types.ts:17) and GitLabWorkItem (src/shared/gitlab-types.ts:165). No schema moves in this commit. It records what main renders for a real row, so the requirement that follows can be read against main's own behaviour rather than against a stub. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): require the members the tasks readers' consumers read unguarded Round-1 review found four unguarded consumer reads at members the corpus proves the host sends, plus two the corrected fixtures now prove. Each one ends in a TypeError inside a render or a useMemo, which is the defect class this migration exists to close. Required, each because a consumer reads it with no guard and the host's own type declares it non-optional: title on an accessible project project.title.toLowerCase() name, directories on a preset localeCompare, and two joins labels on a work-item row item.source.labels.filter, both label editors state.name, team.name, priority createLinearTask, and the reviewer sort All six sit inside a salvagingArray, so a row that lacks one drops and the list survives. The single-row paste lookup names the reply instead, because there is no list for it to survive in. Loosened in the other direction: the SSH record no longer requires `error` or `reconnectAttempt`. Nothing reads either one — the gate spells `matchingState?.error ?? null` and nothing anywhere reads reconnectAttempt — and the record is a salvagedOptional, so requiring an unread member drops the WHOLE record, whose fallback on the connect path is `fallbackSshState(id,'connected')`. A reply of `{ targetId, status: 'auth-failed', error }` would have shown the drawer as connected. Mobile's own stored type is widened to match; the shared wire type is untouched. The `iid` extension on the lookup row goes with it. Neither work-item type declares such a member and every GitLab consumer builds its iid param out of `item.source.number`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the screen parity hashes over the merged tree Main landed the sibling tasks lane, which edits the same screen hook files this branch does, so the merged tree hashes to neither side's constant. Both inputs are legitimate: main's reply-schema conversions and this branch's WorkspaceSshRecord rename. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): justify the smart-source casts by the schema, not the fixture Five SAFETY notes argued from the corpus back to the wire: they justified a cast by the stub rows the fixtures used to carry (`{ iid, title }`, `{ number: 12, title: 'twelve' }`). Those rows were the defect corrected in 6763ff12e9, so the claims are now false, and the reasoning was never sound — a fixture cannot say what the host may send. Each note now cites the schema's own requirement rule, the host type and the consumer read. Comment-only; no golden moves. The hook and statement parity hashes move because `normalized` hashes a statement's full span, comments included. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): repair the line citations this lane's own edits aged Eleven citations across five reply-schema modules pointed at the wrong line. Every one was correct when written and rotted afterwards: the SAFETY-note rewrites, the F4 dedupe's deleted casts and the sibling lane's merge each shifted the files being cited. A citation is the whole argument for a requirement, so a stale one reads as a fabricated one. Found by resolving every `file.ts:line` in the five modules against the merged tree and comparing the line's text to the claim beside it, not by reading them. The ones that still resolve correctly are left alone, including three that looked stale and were not. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the two advisory task probes total so a nullish payload cannot unhydrate the screen `preflight.check` and `linear.status` are read under `success-result-or-skip` and documented as advisory. That policy accepts an envelope whose `result` is absent or null, then asks the reader to decode it; a `looseObject` refuses, the throw leaves the reader, and the caller's catch discards the entire hydration. The corpus records the difference: on the `result-absent` and `result-null` partitions main hydrates the Tasks screen and lists one provider, and the checked readers left it unhydrated with no providers. `.catch` restores main's answer exactly. Every consumer guards to the leaf and compares to `true`, so absence, null and a garbage payload have always meant "not installed" and "not connected". Four cases pin it, and removing either catch fails all four. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC corpus at this lane's head Repins the recording baseline to b354d1338a (the advisory-probe totality fix) and records all 382 scenarios from that tree. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC corpus after merging main Repins the recording baseline to the merge commit and records all 386 scenarios from that tree, so the corpus carries both main's step-7 batch and this lane's. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the Linear row cast the checked schema made unnecessary `found` is already assignable to the mobile `LinearMobileIssue` alias once `linearIssueRowSchema` requires its nine members, so the assertion and its disable line carried no type error. The sibling cast in smart-source-search-requests.ts stays: it targets the shared `LinearIssue`, whose `labelIds` is required where the schema leaves it optional. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): rewrite the SAFETY lines that argued from deleted fixtures Three casts still justified themselves with the `{ id }` and `{ number, title }` rows this branch replaced in round 1, which reads as a licence to loosen the requirements that close the reproduced crashes. Each now names what its schema requires and what the cast actually covers, each verified by deleting the cast and reading the error: - Linear rows: all nine read members are required, so `labelIds` alone is the gap between the schema and the shared LinearIssue. - GitHub search: `items` and eight row members are required; the salvaged `T | undefined` types and the deliberately opaque `sources`/`errors` are what remain. - Sparse presets: `id`, `name` and `directories` are required; the cast covers the three SparsePreset declares that the reply omits. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): resolve the bare :NN citations the first audit could not see The earlier repair resolved only qualified `file.ts:line` citations, and these doc blocks name a file once and then reference it as a bare `:NN`, so every continuation reference went unchecked. Re-running the audit with a resolver that carries the last-named file and directory forward finds twelve stale line numbers and four references whose nearest named file is the wrong one. Fixed: the four `find`/`filter` lines and the layout equality tests under the `views` requirement, the settings commit, the detail refusal throw, and the three metadata guarded reads, all shifted by one to five lines. The ui-state, paste-resolved and host-method references are now qualified, because an intervening citation to another file silently reassigned them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-pin the tasks parity hashes the round-2 fixes moved One statement changed (the Linear list cast is gone) and three SAFETY comments nested inside statements were rewritten, so the hook and statement hashes move. Counts hold at 350 and 417, and the declaration, semantic, render and style hashes do not move, which is what shows no type, call or rendered tree changed with them. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): delete the Linear list reader the checked schema replaced `extractLinearIssueReadItems` lost its only caller when the smart-search operation moved to `rpcResultVariant('linear-issues', …)`. What remained was a function no screen can reach, a suite reporting coverage for it, and a second 'Unexpected Linear tasks response' string competing with the named reply error. The `LinearMobileIssue` type stays: it is the mobile `LinearIssue` alias. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what a refused sparse-preset list actually does to the screen "Reports the named error" overstates it: the error setter's value is destructured with a leading underscore and read by nobody, here and on main. The visible delta is `presetsLoaded` staying false, which disables "New preset" and both draft entry points where main let the user create one. No shipped host reaches the state: `repo.sparsePresets` has no refusal arm. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC corpus after the round-2 fixes Repins to the round-2 head and records all 386 scenarios from it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC corpus after merging main at 7a1f55c52a Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the persisted-ui-state reader total so an unreadable ui reply cannot unhydrate the Tasks screen Main boxed the payload and read `undefined` off a string, number or array, so the screen hydrated; a refusal here threw out of hydrateTaskState and failed the settings, preflight and Linear legs beside it. Null and absent now hydrate with the defaults too, since every read of the state is optional. The GitHub search SAFETY line separates the members the schema requires from the ones it only types, and the parity hashes follow that comment text. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC corpus over the total ui-state reader Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-17 14:08:59 -04:00
Brennan Benson	abc8386e14	fix(mobile): name a create's launch so a lost reply cannot build two workspaces (#21137) ... * fix(mobile): name a create's launch so a lost reply cannot build two workspaces `agent.launch` admits a caller-supplied `operationId` through a durable ledger, so exactly one execution happens and every replay returns the recorded answer. No client sent one, so the machinery was inert and the original defect was still live: mobile retries a lost create by design, and a retried launch built a second agent in a second workspace. Mobile now mints an operation id per create candidate and sends it whenever the host advertises `agent.launch.replay.v1`. The invariant is one operation per candidate. `computeAgentLaunchFingerprint` folds `target` whole, so the workspace name is inside the fingerprint; carrying one id across a name-collision bump would meet its own row under a differing fingerprint and refuse `agent_session_operation_conflict`, failing the create outright on the second candidate. The id is therefore minted beside `clientMutationId` at the top of each loop iteration and reused verbatim by every retry arm inside that candidate — never re-minted, since a new id is a new operation. Admission runs ahead of every effect, so `_invalid` / `_expired` / `_capacity` prove nothing launched: those re-send the same candidate unnamed rather than let bookkeeping fail a create the host would have performed. `_unknown` is the one refusal that is not safe to re-send, and it surfaces. Also corrects a false comment: the legacy path caches the whole launch under `clientMutationId`, so inside its 60s window a replay adds neither a workspace nor a surface, and outside it adds both — not "a second surface, never a second workspace". * fix(mobile): preserve launch identity on refusals * fix(mobile): use launch receipts to authorize replay * test: move mobile launch replay coverage outside node project * fix(mobile): enforce replay-safe launch delivery at the host * test: run mobile launch contracts in mobile checks * test: cover mobile launch contract workflow dependencies	2026-09-17 10:06:11 -07:00
Jinwoo Hong	3e32b83522	refactor(mobile): checked reply readers for notifications, components, terminal, transport, home, worktree and browser (step 7) (#21176) ... * refactor(mobile): checked reply readers for notifications, components, terminal, transport, home, worktree and browser (step 7) Twenty-one unchecked reply readers across thirteen files become checked zod readers, so a malformed host reply surfaces as one `RpcIncompatibleReplyError` naming the method instead of a downstream `TypeError`, a rendered `undefined`, or a card left "proven" over a reply that carried no rows. Deliberately a behaviour change on malformed replies only. What each domain required, and why it required no more: - notifications (5 readers). All four call sites read the payload through `?.`, so every schema is nullish at the top level and no member is required. The test-push `reason` and the register `reason` become closed enums, because the two comparisons against them are the whole of what they decide and an arm this build does not know took the generic copy on main too. The stream unsubscribe and the unregister read no body at all. - components (4). `repo.hooks` requires `source` and nothing else: the drawer assigns it straight into `SetupHookDetails.source`, whose type is `string | null`, with no guard in between — nullable so the "no hooks file" answer keeps its explicit null. `setupTrust` is nullable as well as optional because the `components-setup-ask` fixture sends an explicit null, and salvaging that would move a `normal` golden. `ui.get`'s trust record salvages per repo, so one unreadable repo cannot cost the others their approvals. The Codex redeem reply stays `z.unknown()`: `decodeResetResult` is a real scope-and-snapshot validator and splitting it would give one reply two refusal rules. - terminal (4). The send verdict and the viewport pair keep main's exact `=== true` projections. `terminalSendAcceptedSchema` moves here from the session domain, which now re-exports it: terminal is the lower layer and two identical copies could drift on what "delivered" means. `terminal-send-rpc-response.ts` is deleted, its projection now being the schema's. - transport (3). `status.get` declares its five members and requires the object; the three callers disagree about what an unreadable status means, so each keeps its own verdict behind a named reader — the gate wants the failure, and the probe and the pairing race must not have it, because both call `interpret` inside a `.then` fulfilment handler where a throw becomes a detached rejection. `capabilities` salvages whole rather than per element, which is main's own rule and what `transport-capability-probe-non-string-capabilities-drop` records. The two pairing readers are the shared credential contract itself, moved off the four call sites that each ran `.parse()` on the interpreted value; its `.strict()` is main's shipped rule for that released surface, not a new one. - home (2), worktree (2), browser (1). The stats row is checked as an object and nothing more, `totalHomeStats` being the reader that says so itself; its per-host slot is now typed as the wire row it holds rather than as the computed total. `worktree.ps` cannot require `worktrees`: the host answers a union whose unchanged arm carries `{ unchanged, snapshotId }` and no rows. The twelve browser commands read no body; `browser.goto`'s settled URL stays nullish because `navigateToAddress` is inline in `MobileBrowserPane.tsx`, which no adapter mounts, and a move there would ship unevidenced. Three fixtures were wrong and are corrected, each disclosed rather than worked around: the runtime-context test kept a content hash directly under a repo key, which is not a shape `ui.get` sends; and two snapshot-client tests ran their reply list dry and handed `fetch` an absent result while claiming to model a transport failure. `push-test-envelope` is re-anchored at the same defect's new home, the cast having been deleted. The boundary test's offender floor comes down from 20 to 10 with the list, which is what its own comment says it is for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus and re-record step 7's checked reply readers `baseline` moves to this branch's product commit, which is what `--record` compares the fenced tree against, and every one of the 758 goldens is re-recorded from it. The repin is what rewrites the `baseline` header on all of them; nothing else about the corpus moves except the bodies disclosed below. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): mutate the workspace catalog's reader back to unchecked The step-7 defect evidence needs a scenario whose reply is the one the change moves. Every pilot scenario in the catalog family scripts a well-formed reply, so a mutant that only changes how a *malformed* reply reads has nowhere to diverge — which is why the pilot's own suite passed against an unchecked catalog reader while its matrix golden failed. `worktree-catalog-snapshot-unreadable` scripts `worktree.ps` answering `{ ok: true }` with no result at all, which is what `result-absent` drives at the matrix site, and records the fetch rejecting with `RpcIncompatibleReplyError`. `worktree-catalog-unchecked-reader` then swaps the operation's reader for one that answers `compatible: true` for every payload — main's reader, in one line — and the recording moves back to a fulfilled fetch carrying `admission: { kind: 'invalid' }`, which is the answer that let a broken catalog render as an empty host (STA-3123). One golden added and none moved: the manifest sits outside the fenced paths, the family's matrix base is still `worktree-catalog-snapshot`, and the mutation registry is not part of `recorderSha256`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the push-test reason arms the closed enum constrains `pushDeliveryTestResultSchema.reason` closes over the four arms of the host's `MobilePushTestResult` (src/shared/mobile-push-contract.ts:99), but no scenario carried the member, so the corpus could not have caught a wrong vocabulary. Three scenarios on the existing display-test mount carry it now: the two arms the screen branches on and one arm no build knows. Each golden was recorded first at the main pin 4b876758d3 and reproduces there byte for byte, so the `normal` partition pins main's rendering rather than this branch's. The unknown arm is the load-bearing one: main renders "Could not send through Orca's push service." for an unrecognised string, and the salvage drop renders the same sentence, so the closed enum costs a recorded `reply-salvage` observation and no screen text. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): cite the host union behind each closed reply enum A closed `z.enum` is only as good as the vocabulary it was checked against, and mobile's own declared types are written from memory. Each of the three enums now names the host type it mirrors, so the next reader re-checks it in one grep rather than trusting the arms. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the commit that carries the host-union citations `--record` compares the fenced tree against `baseline`, so a comment in `mobile/src` moves the pin like any other product edit. Every one of the 762 goldens changes by exactly its `baseline` line and nothing else, which is the evidence that the citation commit is inert. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the agent-history panel rendering when the catalog omits its rows `worktrees` is a salvaged member, so it is absent on the three envelopes that read as an object without rows, and `use-mobile-agent-history-state.ts:61` calls `.find` on it with no guard. The cast erased that and the corpus recorded the result: `matrix-aivault.history-screen-worktree.ps-1` carried `crash: Cannot read properties of undefined (reading 'find')` on `inner-ok-missing`, `inner-false-string-error` and `inner-false-object-error`. `?? []` is what the sibling Home card already does. The SAFETY note cited that card's golden, which is the opposite site, and now cites this panel's own family. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep a malformed worktree.ps reply reported as invalid_response `host-workspace-list-states.tsx` prints this code to the user verbatim, so the checked reader's throw landing in the generic catch renamed a host-payload defect into a connectivity failure. STA-3123 exists to make a broken remote host diagnosable, which `network_error` is not. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): give the browser page commands back their one-line form Only `browser.goto` reads a reply body, so the reader belongs on a two-argument wrapper rather than on all thirteen call sites. The exported type of every command is unchanged, and the doc comment no longer promises a shape the file did not have. 109 lines to 72. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): import the terminal send verdict from the terminal domain The re-export forwarded to two callers, which a direct import already gives them, and the session suite re-pinned three cases `terminal-reply-schema.test.ts` owns. One definition, one pin, one file hop fewer. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): move the codex reset-credit pins next to their schema They sat in the New Workspace suite, so a reader looking for the capability whole-list drop by filename did not find it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record after the round-1 fixes Three checkpoints of `matrix-aivault.history-screen-worktree.ps-1` move from a recorded white screen to the loading list: `inner-ok-missing`, `inner-false-string-error` and `inner-false-object-error` answer the panel an object with no rows, and the `?? []` now seats an empty list instead of letting `.find` throw. Every other golden changes by its `baseline` line alone, which is the evidence the other four fixes move nothing the recorder observes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop requiring the stats row its own reader guards Round-2 findings 1 to 3. `homeHostStatsSchema` required an object that `totalHomeStats` already guards (`if (!host || typeof host !== 'object') continue`), so the requirement bought nothing at the read and cost the row upstream: the refusal reached `fetchMobileHomeStats`'s `.catch`, the per-host slot was never written, `hostIds.filter` found no host and the Home header drew no stats row where main drew `0 / 0s / 0`. It takes `.nullish()`, and `HomeStatsRow` admits the `null | undefined` main always had. The unit pin now says the slot keeps a null summary and the total skips it, and sums one through `totalHomeStats` to show the zeroed row survives. The Home card's `SAFETY:` note claimed the reader proves `worktrees` is an array. It does not; the `?? []` does. That is the same false sentence round 1 removed from the agent-history panel, and a reader who believed it would delete the `??` and reintroduce the white screen. The `catalogError` branch on `RpcIncompatibleReplyError` had nothing holding it: no adapter mounts the host screen, so no golden can reach it. One case in the snapshot client pins the class the `catch` keys on. Mutation-checked by forwarding the catalog schema as `z.unknown()`, which fails that case alone. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record after the round-2 fixes and main's merge Pin moves to 0b8bd1c3c7, this branch's last fenced commit. The re-record normalises the ten session create-terminal goldens main re-recorded in #20069 without repinning, and writes the Home stats family's bodies back to what main records now that the summary reader no longer requires the object its own consumer guards. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record the corpus at the merge with main The merge took main's goldens verbatim at main's pin, so the four goldens this branch adds were the only ones whose header did not name a commit in this history. Repinning to the merge commit and re-recording gives all 764 one pin and one recorder, which is what the new ancestry guard asks of the corpus. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): point the reply schemas' citations at the lines they claim Every `file:line` in this branch's diff was resolved against the tree and its target line compared to the claim beside it. Fourteen were wrong. Most had drifted one to four lines onto a closing brace or the line after the code they quote, which reads exactly like a good citation and can only be caught by resolving it. Two were wrong in kind rather than by a few lines. The viewport comment credited the `=== true` projection to the refit call site, which reads plain truthiness; the rule was main's reader, and the refit's two decisions are the lines now cited. The capability comment cited a line for main's `every(typeof === 'string')` rule inside the code this change deletes, so it resolved to an unrelated brace; it now states the behaviour and says why no line carries it. Two more pointed at the head or tail of the statement they named and are tightened to the line that does the work. Comment-only: no schema, no reader and no call site moves. The corpus is repinned and re-recorded on top because the recorder fences `mobile/src`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record after the citation fixes The citation commit is comment-only, and the corpus says so: all 764 goldens move on the `baseline` header line and nothing else. The re-record is needed only because the recorder fences `mobile/src`, which a comment is inside. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the three unguarded-site reply schemas total so a malformed result cannot throw where main tolerated it The ui.get reader is interpreted inside a fire-and-forget IIFE with no catch, and the home stats and push delivery readers sit behind handlers that would print the reader's own sentence where main drew a zeroed row or the generic copy. Each schema now decodes any unreadable result as absent, which lands in the fallback main already took. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record the corpus over the total schemas Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): name the host-status gate reader by its export Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-17 12:40:18 -04:00
Jinwoo Hong	4b876758d3	refactor(mobile): checked reply readers for the session domain (step 7) (#21089) ... * test(mobile): record main's session reply behaviour at every unrecorded read site Step 7 for the session domain changes how 51 RPC readers read a *malformed* reply. Eleven of the session read sites had no recording family, so main's answer to a malformed reply at those sites was undocumented and the reader change would have had nothing to move. This commit is the before picture, taken from main's own tree with no product edit in it. Ten new families, twelve scenarios, twenty-five goldens: - `session.review-file-diff` / `session.review-branch-diff` — `git.diff` and `git.branchDiff` read through the review projection, which the Changes screen's verbatim readers do not cover. - `session.review-git-mutations` — the single-file `git.stage` / `git.discard` and the bulk stage sweep's second `git.stage`. - `session.review-send-sheet` — `session.tabs.list` read for the agent terminals the send sheet lists, the third reader on that method. Needs an `open-send-sheet` action on the review-action adapter, which re-digests that family's eight goldens on `adapterSha256` and nothing else. - `session.browser-tab-create` — `browser.tabCreate`. - `agentSession.structured-create` — `agentSession.create`, whose family base only ever covered the support probe. - `session.tab-rename` / `session.tab-close-session` — `terminal.rename` and `session.tabs.close`. - `settings.new-tab-local-agents` — `preflight.detectAgents`, the arm the new-tab loader takes for a workspace with no connection. `baseline` is repinned to main's tip because two commits (#20659, #21004) touched a fenced path after the pilot's pin, so `--record` refuses on main's own tree until it moves. The repin is what rewrites `baseline` on all 705 existing goldens; nothing else about them moves. Decoded against origin/main through the value pool: 705 header-only (`baseline` on every one, `adapterSha256` on the eight review-action goldens), 0 body-moved, 25 added, 0 deleted. Not covered, with the reason: the chunked clipboard upload's `appendImageUploadChunk`, `commitImageUpload` and `abortImageUpload` cannot be matrixed, because `replyMatrixSites` takes every completion in the base scenario and the chain's later params carry the `uploadId` the start reply named. Driving `clipboard.startImageUpload#1` therefore makes main send an append whose params no scripted step matches, and the recorder raises `Request params mismatch: clipboard.appendImageUploadChunk#1` instead of recording. The two families were written, probed and removed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): checked reply readers for the session domain (step 7) Fifty-one unchecked reply readers across nine files become checked zod readers, so a malformed host reply surfaces as one readable error at the operation boundary instead of a downstream `TypeError`, a rendered `undefined`, or a screen left ready over garbage. Deliberately a behaviour change on malformed replies only. Eight schema modules, one per reply family, each recording the consumer line behind every requirement and the host handler that publishes it: - `clipboard-image-reply-schema.ts` — the upload slot's `uploadId`, the commit and single-frame path strings, and the two legs whose body nothing reads. - `github-pr-mutation-reply-schema.ts` — the `{ ok, error }` status envelope as two variants, and the bare-boolean confirmation. - `github-pr-entity-reply-schema.ts` / `github-pr-read-reply-schema.ts` — the seven PR sidebar reads. Every identity requirement the hand parsers had is kept, so a payload that degraded to null still degrades to null; what changes is a payload that is not the declared container at all. - `diff-review-reply-schema.ts` — the normalized branch compare, the review notes on the worktree record, the three file-diff arms, and the file-level git mutations. - `review-terminal-reply-schema.ts`, `session-launch-reply-schema.ts`, `session-read-reply-schema.ts`, `session-write-reply-schema.ts` — the review send sheet, the launch paths, the session screen's reads and its writes. Requirements are exactly the members a consumer reads unguarded, everything else is a salvaged optional with main's own default applied in the transform, and no schema is `.strict()`: a member a newer host adds passes through untouched. Enum arm sets that a reader compares against pass through or degrade to the arm the reader handles most conservatively; the two closed sets — the committed change status and the diff kind — are closed because main *dropped* an arm it did not know rather than passing it through, and degrading them would draw a row or render a diff main never did. No member is coerced on the way back to the host. `github-pr-parsers.ts`, `github-pr-comment-parsers.ts` and `github-pr-value-readers.ts` are gone; their suite is now the parity record for the schemas that replaced them, with the four cases that refuse rather than degrade marked as such. Twelve call-site casts are deleted, and three dead "response was invalid" branches with them: the reader refuses those replies now, so the error names its method. The nine session files come off `unchecked-rpc-reader-inventory.ts` entirely rather than being lowered. `git show --stat` on this commit touches nothing under `mobile/rpc-foundation`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): unit-pin every session reply schema's decision Three kinds of case, one per kind of decision the schemas encode: a member a consumer reads unguarded is required and its absence refuses, an arm set a reader compares against degrades to the arm that reader handles most conservatively, and a reply whose arms need different members is declared as variants and each arm is read. The last suite is the wire-compatibility claim: a member no reader knows passes straight through, on the markdown document, the upload slot and the terminal inventory alike, so a newer host is never refused for a field mobile does not read. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh the corpus for the session domain's checked readers Repins `baseline` to the last commit touching a fenced path and re-records all 730 goldens, which is the disclosed behaviour change taken as an observation. Decoded through the value pool against the pre-refactor tree on this branch: 688 header-only with `baseline` the only key that moved, 42 body-moved, 0 added, 0 deleted. The 42 are seven named scenarios and thirty-five matrix goldens, and every moved checkpoint's own reply is malformed or refused. Three `normal` partitions appear in the list and none of them reads a well-formed reply differently: the review file-diff family's base scenario drives three legs and its third is scripted `{ kind: 'unknown' }`, so that leg's checkpoint moves in every variant, the varied leg included. The same append-only-history effect puts `pr-read-upstream-error`'s `no-pr` checkpoint in the list for the malformed PR recorded before it. What the corpus now records, in one sentence: a property read on null, a V8 destructuring message shown to the user, and four hand-written "response was invalid" strings are replaced by one message that names the method, and four screens that published a malformed payload as ready state now show an error instead. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): split the expanded check run out of the PR read schemas `github-pr-read-reply-schema.ts` was 328 code lines against the 300-line cap. The expanded check run and the annotations, jobs and steps listed under it are one reply with no reader in common with the other six, so they move to `github-pr-check-reply-schema.ts` whole. A move, not an edit: no schema changes and no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the branch's last fenced-path commit The schema-module split touched `mobile/src`, so `--record` refuses on the pin the previous refresh left behind. Repins to that commit and re-records. Decoded against the previous corpus: 730 header-only with `baseline` the only key that moved, 0 body-moved, 0 added, 0 deleted — the split is a move, and the corpus says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the worktree display-name cast's type import The live-title read is typed by its schema now, so the cast it annotated is gone and the import it needed with it. oxlint flags the leftover. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the branch tip The unused-import removal touched a fenced path, so the pin moves with it. Decoded against the previous corpus: 730 header-only on `baseline` alone, 0 body-moved, 0 added, 0 deleted. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): contain a refused prChecks reply to the checks section The checks read was the one phase-1 dependency that could take the whole PR sidebar down. `loadPrSidebarData` routed `!checksOutcome.ok` through `failureState`, so a host whose `github.prChecks` shape drifted cost the user the title, body, comments, reviewers and merge controls — everything they opened the sidebar for — over a section that renders a row of icons. Main never noticed because its unchecked reader answered `[]` for the same reply; this branch's reader refuses it, which is correct, and which is what makes the containment necessary. Contained the way phase 2 already is: a failed read keeps `kind: 'ready'`, empties `checks`, and carries the message in a new `checksError` so the checks section can say what happened. The sidebar can no longer reach `error` or `blocked` on the checks read alone. Also pins the enum departure this PR makes deliberately. The degrading arm sets go through `salvagedOptional(name, z.enum(...))` rather than `openEnum` because `openEnum` refuses a non-string where main mapped it to the conservative arm; nothing held that, and all 2477 tests stayed green against the swap. Six cases now hold both halves: a non-string degrades on the three open sets, and an unknown arm drops the row on the closed ones. Four deletions the reviewer found: a reaction-token alias with no importers, the `errorType`/`fetchedAt` the branch-lookup reader fabricated to satisfy a type whose only consumer reads neither, two bare schema aliases, and a quick-commands pass-through with two callers. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the containment commit `--record` refuses unless the product tree equals `baseline`, so the fix above moves the pin. The corpus re-recorded in place against it: 730 goldens, every one header-only on `baseline`, no observation moved. No observation moved because no family reaches the code the fix changed. The `github.pr-read` family calls the seven wrapper reads directly and records their `{ ok, error }` outcomes; `loadPrSidebarData` sits a layer above that and no scenario mounts it. The prChecks outcome is identical before and after — what changed is what the sidebar does with it — so the unit suite is the only oracle for the containment. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the PR sidebar's checks containment The containment landed with no golden: no scenario mounted `loadPrSidebarData`, so the row in the delta table rested on unit tests alone. `PrSidebarLoadDeps` is five client-taking functions, so a new adapter drives phase 1 directly and records the `PrSidebarState` it resolves to — no React host, and no edit to an existing adapter, so no recorded golden moves. Two scenarios: a normal load, and one whose checks leg answers a shape the reader refuses. The matrix over the base then drives all eleven partitions at `github.prChecks#1`, and every one of them records `ready` with a `checksError` where main took the whole sidebar to `error`. `pr-sidebar-checks-failure-state` is the mutant that routes the refusal back through `failureState`; it moves both `pr-sidebar-checks-refused` and the prChecks matrix golden. Also pins two closed-and-required enum decisions that were free to become defaults — an unknown check-summary state drops the summary block, an unknown reaction content drops the reaction — deletes four exported type aliases and five enum constants with no reader outside their own file, makes `PRChecksSection`'s `checksError` required so a second caller cannot silently lose the message, and stops the header reading "No checks" when the checks were unreadable rather than absent. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the pr-sidebar family commit Six new goldens — two pilots and the four matrix sites the base scenario scripts — and `baseline` on the 730 that already existed. No body moved and no `adapterSha256`: the family is a new adapter module, so nothing recorded through another one re-digests. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the corpus against the merged main Repins `baseline` to the merge commit and re-records all 736 goldens in place. Against `origin/main` the 705 shared goldens move only on `baseline` (672 of them header-only), leaving the same 33 body moves and the same partitions the branch carried before the merge, plus its 31 added goldens. Every body also takes main's recorder shape from #21088: `sent` becomes `ordinal` over one interleaved write counter, subscriptions record a cleanup checkpoint, and a salvaging read now reports a `reply-salvage` effect naming what it dropped. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep an explicit null on the two tri-state PR flags `autoMergeAllowed` and `mergeQueueRequired` carry three answers, not two: `null` is GitHub saying auto-merge is not allowed, `undefined` is the host not carrying the member at all. The readers coalesced the null away, so a well-formed reply read differently from the parsers they replaced, which preserved it explicitly. Both shared types already declare `boolean | null`. No consumer separates the two today — `pull-request-auto-merge-availability` compares with `=== true` and `!== false` — so this is parity, not a visible fix, which is exactly why it needed a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the corpus to the tri-state flag commit All 736 goldens move on `baseline` alone: no scenario scripts an explicit null on either flag, so preserving it changes no recorded screen. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): check the two session-write readers #21083 brought Step 7 empties the session block of the unchecked-reader inventory, and #21083 landed two readers into it after that: the New Tab create's member read of `tab`, and the display-mode toggle's payload. Converting them is what keeps the claim true — a session line reappearing would mean the domain is not migrated. `created-terminal-tab` requires `tab.id` and `tab.type === 'terminal'`, because the strip keys the new tab on the id and spreads the rest into a union whose arm `type` picks. `terminal`, `title` and `terminalTheme` stay optional behind main's own guards, and unknown members pass through. `terminal-display-mode-set` reads nothing, so it takes the same `z.unknown()` the other five unread writes take. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin and re-record over #21083's corpus All 736 goldens this branch already had move on `baseline` alone, and #21083's 22 arrive beside them. One of the 22 moves against main's own recording: `matrix-session.create-terminal-session.tabs.createterminal-1`, where the New Tab create's five malformed partitions read `Cannot read properties of undefined (reading 'tab')` and now read the method's own message. Two of them also stop unsubscribing the terminal the user was watching before the property read threw, so a create that never happened no longer costs the live pane its subscription. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what carries a refused create reply to the catch Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-16 21:24:31 -04:00
Brennan BensonandMerge Sim	97aa5ff19b	fix(mobile): open native chat when a new worktree launches a default agent (#19850) ... * refactor(agent-launch): make the launch-mode decision surface-neutral `decideWorkerStartMode` was the only shared answer to "structured chat session or terminal agent?", but it lived in an orchestration-named module and spoke orchestration's vocabulary, so the other launch surfaces could not call it. Move the decision to `main/agent-launch/agent-launch-mode` unchanged and leave `orchestration-worker-start-mode` as the adapter that supplies the noun. A worker is not a special kind of launch; it is the same launch with a dispatch attached. Naming the receipt's subject is the only thing orchestration actually contributed, so that is the only thing the adapter keeps: "worker" in both sentences, plus the `--terminal` wording, which reads as nonsense anywhere a `--terminal` flag does not exist. Both are pinned, because they are asserted. No behavior change. The receipts are byte-identical for every reachable case, proven by running the new pin against both implementations. Also pins the wording, which nothing was holding. The existing suites assert `toContain` fragments ('terminal agent', 'cannot create') and the CLI suite asserts a receipt handed to it by a mock rather than one this code produced; all six files stayed green against a deliberately corrupted vocabulary. A dispatch receipt is the only place a structured-to-terminal downgrade explains itself, so the whole sentence is the contract, not a fragment of it. * feat(agent-launch): add the launch intent and the one executor that runs it The sequencing around the launch decision was duplicated per surface, and the duplicate is where the bug lives. A new worktree was created agent-first, so its startup terminal WAS the agent and the structured branch below it could never be reached — every new-worktree launch was a PTY regardless of the user's default. Orchestration fixed that for itself in #19431; mobile and the CLI still have it. `executeAgentLaunch` inverts the order once, for everyone. When the preference is structured the worktree is created with NO startup agent, the executing host is then asked whether it can host a session for the workspace that now exists, and only then is a surface created. The host verdict cannot be hoisted above creation: `agentSession.createSupport` only answers for a workspace it can resolve, which is why the decision stays in two halves. Agent-first creation is deliberately preserved for PTY launches — it is what sequences the agent's startup command behind the setup runner, so wait-for-setup comes for free there. What actually differs per surface is only how a surface is built (an orchestration worker's session takes a dispatch hold and a mailbox a plain launch must not take), so that is injected as a factory rather than branched on. The intent also strips the reserved agent fields from a migrated create payload: a caller moving off `worktree.create` passes its existing params, and a stale `startupAgent` in there would re-create the very path this replaces. Tests assert order and arguments, not just the resulting mode. Reintroducing agent-first creation reddens 4 of 11. * feat(agent-launch): expose the launch executor as the agent.launch RPC Adds `agent.launch` — one host-side method that decides structured-vs-terminal and creates the surface — wired to the real runtime factories: `createManagedWorktree` for the workspace, forking on `startupAgent` exactly as the orchestration worker path does; `createStructuredAgentSessionForWorktree` for a chat session; and `createTerminal` for a PTY agent. Allowlisted for mobile, which is the surface the routing gap was reported on. `worktree.create` is untouched. Its `startupAgent` keeps meaning "spawn a PTY agent" verbatim, because it answers with `agentTerminalHandle` only on that path: a host that quietly routed it to a structured session would hand every older client a response with no handle and no error. All new behaviour sits behind `agent.launch.v1`, which the host now advertises and a remote client must negotiate, so a client that does not gets today's behaviour unchanged. * feat(mobile): route workspace creates through agent.launch Picking an agent on the mobile create sheet always produced a terminal, even when the user's default was native chat, because all three create paths put `startupAgent` on `worktree.create`. That means "create the worktree agent-first", so its startup terminal IS the agent and the structured branch below it is unreachable — while the same phone's in-workspace "+" button opened a chat. The blank, branch and new-branch creates now send the same payload through `agent.launch` and let the host settle the surface. `worktree.create` is untouched, and a host that does not advertise `agent.launch.v1` (read from the existing `status.get` probe) keeps today's path exactly. Work-item creates stay on `worktree.create`: they pre-fill the issue/PR URL as an unsent `startupDraft`, which a structured session cannot hold yet, so routing them would submit the URL as a first turn. * fix(agent-launch): drop the deleted draft-prompt blocker from the reason map main removed the draft-prompt blocker in #19681 (a structured session now holds an unsent draft), so the exhaustive Record no longer typechecks. * chore(agent-launch): carry a SAFETY rationale on the agent placement cast The type-assertion gate landed after this branch's base, so the new file's copy of the worker-start cast is now a changed-code finding. * chore(agent-launch): carry agent.launch through main's RPC typing and casting gates The typed-method contract, the generated params catalog and the `assertionStyle: never` casting scan all landed after this branch's base. - AGENT_LAUNCH_METHODS kept an `RpcMethod[]` annotation, which widened its method name to `string` and broke assignability; every sibling infers instead. - `agent.launch` binds a schema under src/main, so it joins the catalog's RPC_METHODS_WITHOUT_SHARED_PARAMS and the parity gate's hand-listed twin. - The now-typed methods make most test casts unnecessary; the few that remain carry the line-specific SAFETY rationale the casting gate requires. * test(mobile): supply the agent-launch fixture the create-submit recording needs The golden RPC recordings landed upstream while this branch was out, so they first met agent.launch here. Three things had to happen, and only one of them is a fixture bump. 1. workspace-settings-mounts.ts mounts useNewWorkspaceCreateSubmit against a fixture model that throws on any member it was not given. This PR added a required getAgentLaunchSupport, so the submit aborted with "Missing model fixture" before it ever issued the create, and three cleanup checkpoints vanished. That read like a product regression and was not one. Supplying the member restores the recording byte-for-byte; it is pinned false for the same reason the cutover probe is, so the baseline stays on worktree.create. 2. Editing that adapter moves adapterSha256 for the twelve settings goldens it mounts. Their recordings are unchanged - header only, by design: the digest is per-golden so editing a module fails exactly the goldens that mounted it. 3. Five goldens changed behaviourally, and both changes are this PR's: the capability probe now reports agentLaunch, and a create whose reply carries no worktree returns "Failed to create workspace" instead of throwing a TypeError off an unguarded result.worktree read. The launch route needs that guard, since a receipt can arrive without a worktreeId. * refactor(mobile): decode the launch receipt instead of asserting its shape The changed-code quality gate refuses type assertions, and the eight it flagged were worth removing rather than suppressing. The production one was the point. readAgentLaunchCreateOutcome asserted the RPC payload into Partial<AgentLaunchResult> and then runtime-checked it anyway, so the assertion bought nothing and claimed a contract the host had not proven. It now narrows with `in` and validates each hop, which is the same nullability question readCreateResult already answers on the sibling path - a launch receipt can legitimately arrive without a worktreeId. AgentLaunchCreateOutcome ties worktreeId to the shared contract so a change there fails this reader's typecheck rather than passing a differently-typed field through. The test fakes claimed a whole RpcClient via `as unknown as RpcClient` while implementing one member. They now build a typed literal, matching the pattern in use-mobile-structured-agent-options.test.ts. The read sites cast params and then read one field; they now assert the payload with toMatchObject, which removes the cast and pins more of the shape than the cast did. Also pins the warning passthrough, which nothing covered: a terminal launch that seats the workspace but cannot start the pty reports why, and the absent, blank, non-string and structured-surface cases report nothing. Writing that test caught a real drop I had introduced in the reader. * ci(mobile): re-run Mobile Checks when a shared capability changes Mobile Checks is path-filtered to mobile/**, but mobile imports the negotiated capability names straight from src/shared/protocol-version.ts and records the whole capability read verbatim in its goldens. So a capability added desktop-side rewrites a mobile fixture while never triggering the suite that would catch it. That is what happened here: #19849 introduced agent.launch.v1 and Mobile Checks never ran on it. Verified at the run level rather than by check name - the window-free check-runs API on 3837ae8d51 returns 49 check-runs across six runs (PR Checks x2, PR test LoC x2, Track Community PRs, Review) and no Mobile Checks among them. The breakage surfaced only in this PR, which happens to touch mobile/**. The workflow already concedes this pattern for terminal-file-link-conformance.ts; protocol-version.ts has the stronger claim, since mobile records its output. Also corrects the mount adapter's SAFETY comment. It claimed the recorder supplies only the members the hook reads, which was false the moment the hook gained a required getAgentLaunchSupport - and the assertion it annotates is exactly what stopped the compiler from saying so. The twelve goldens are adapterSha256 churn from that comment: every body is byte-identical, which is the digest doing its job. * docs(agent-launch): stop the receipt-wording comment claiming a migration The decision was never moved out of orchestration-worker-start-mode; this PR adds a second copy beside it. Say so, and name the unenforced agreement. * docs(agent-launch): stop the executor comment claiming a migration that has not happened The header asserted two things the tree does not support: that every launch surface routes through the executor, and that the mode decision "already lived" in `agent-launch-mode`. `agent.launch` is the executor's only consumer, and `orchestration-worker-start-mode.ts` is byte-identical (blob 92dc5c644a, 217 lines) at the merge base and all three stack heads, still used by workers.ts. Describe the two live copies and leave the cutover to later stack work. * fix(agent-launch): preserve setup and refusal fallbacks * refactor(mobile): parse the launch outcome into a named type at its boundary anti-slop/no-object-parameters flagged terminalLaunchWarning's `result: object`. The rule is pointing at a real seam rather than a style nit: the helper advertised a loose object and did the narrowing inside itself, so every caller handed it unparsed wire data and nothing downstream held a real type. Parsed at the boundary instead. parseTerminalLaunchOutcome takes `unknown` and returns TerminalLaunchOutcome | null, so the narrowing happens once, where the untrusted payload enters, and the consumer works with a named type. The type is taken from the shared contract rather than restated - a Pick over the terminal member of AgentLaunchOutcome - so a change to that union fails here instead of flowing through. `handle` is deliberately excluded: nothing reads it, and requiring it would drop the warning off a reply that omitted one, which is a behaviour change smuggled in under a typing change. No assertion and no config exemption: reintroducing `as Partial<AgentLaunchResult>` would trade this finding for the defect removed earlier in this branch, and the rule is correct here. The rule arrived with the merge-forward (#20781, newer than this branch's merge-base), and anti-slop is not one of the changed-code gate's six scans - it runs only repo-wide - which is why a clean local gate did not predict it. Behaviour is unchanged across all five warning cases, and the positive case was re-ablated on the new parser: dropping the warning reddens exactly it, 1 failed | 18 passed, restored byte-identical to 19 passed. * fix(agent-launch): dedupe complete launch and cancel setup wait * fix(agent-launch): memoize the whole launch so a replay cannot mint a second session A replayed agent.launch could create a second structured session in the same worktree, with activate: true. dedupeWorktreeCreate wrapped only the worktree half, inside the workspace factory. On a replay the create was reused, and the executor then continued to createSurface and built another surface inside it. The terminal route hid this: its cached create carries a startup terminal handle, so the executor returns on early. A structured create has no handle by construction - that is the whole point of the structured fork - so it fell through every time. Mobile replays this method deliberately on a delivery-ambiguous response, up to five attempts, so the path is reachable by design rather than in theory. The handler now wraps the entire launch in the same dedupe, on the same (repo, clientMutationId) identity, exactly as worktree.create wraps its own body. A replay returns the original AgentLaunchResult instead of re-running createSurface, which makes the two routes replay-identical. The inner dedupe is removed rather than kept. Wrapping both levels on one key deadlocks: dedupeWorktreeCreate stores the in-flight promise before the inner call runs, so the inner call would be handed the outer's promise, which is waiting on it. The launch-level memo subsumes the worktree-level one. Failures are still dropped rather than cached, so an unknown outcome stays unknown instead of replaying as a fabricated success. The guard replays a STRUCTURED launch: the terminal route cannot reproduce this and a test there would pass either way. Ablated against the pre-fix files - 1 failed | 22 passed, "expected vi.fn() to be called 1 times, but got 2 times", which is the duplicate session - then restored to 23 passed. The stub's dedupe had to be made faithful for that to be observable; the shared one passes through so other tests can see raw calls. * Revert "fix(agent-launch): memoize the whole launch so a replay cannot mint a second session" This reverts commit 59bc5e9b04. The same defect was already fixed upstream on this stack's base branch by 539e283c0f, which landed while this was being written. That change is broader (it also cancels the setup wait) and namespaces the dedupe key, so it supersedes this one. Reverting rather than hand-merging keeps a single implementation instead of a hybrid nobody chose. The behavioural guard from this commit is ported back on top of the upstream implementation separately: it asserts exactly one structured session survives a replay, where the upstream tests assert the dedupe wiring. * ci(mobile): close the round-1 signal gaps around agent.launch Three review findings, all narrow. Mobile Checks is path-filtered, and this branch made mobile's types depend on the shared RPC contract: rpc-params-contract.ts is a type-only re-export of the generated params catalog, and mobile/tsconfig.json includes **/*.ts. So a desktop-only edit under src/shared/rpc-contract/ could break mobile's typecheck with no mobile signal at all - the same blind spot the protocol-version.ts entry closed, one directory over. Added src/shared/rpc-contract/** to the paths filter. agent.launch had no cross-version trigger. Added the three prefixes a paired peer actually exchanges: the intent contract, the wire schema, and the RPC method. src/main/agent-launch/ is deliberately NOT listed - the executor shapes behaviour but is not itself wire, and AgentLaunchResult's shape is already covered by agent-launch-intent. Extending the cross-version SUITE to cover a negotiated handshake is separate work, not this. The break branch that answers an accepted-but-empty reply with "Failed to create workspace" had no unit coverage; the golden that used to discriminate it collapsed five partitions into one shared error when the null guard replaced the unchecked read. Covered on BOTH routes - worktree.create with no worktree.id and agent.launch with no worktreeId - since the branch serves both. Ablated by bypassing the guard: 2 failed | 11 passed, the two new cases returning a fabricated worktree instead of the error, restored to 13 passed. * fix(agent-launch): give a launch one place to say the workspace is incomplete createManagedWorktree reports an unspawned startup terminal or an uncopied working tree as a top-level `warning`, and worktree.create hands it straight to mobile. The launch path narrowed that result down to {worktreeId, startupTerminalHandle} and dropped it, so every agent.launch create lost a warning the old method surfaces - on both arms. The channel was also asymmetric by accident rather than design: a terminal outcome could carry `warning`, a structured one had nowhere to put it, so the arm this PR exists to enable was the arm that could not report an incomplete create at all. Now there is exactly one place a launch warning lives: AgentLaunchResult.warning, at the top level. It is about the create as often as the surface, it applies to a structured session and a terminal alike, and a reader should not branch on outcome.kind to discover the workspace it just opened is missing something. The terminal arm's own `warning?` is removed rather than left beside it - two homes for one fact is how they drift. Every producer folds in: the create, the surface, and the refusal downgrade. Consumer census before removing it: one production reader (mobile's readAgentLaunchCreateOutcome) and no others - the renderer and mobile launch call sites never read it. The mobile reader now reads the top-level field, which also lets its outcome parser go away entirely. Guard ablated by restoring the pre-fix narrowing: 2 failed | 24 passed, both carriers reporting `expected undefined`, which is the dropped warning itself; restored to 26 passed. The third case asserts an absence and stays green under the mutation by construction - it pins shape, not the defect. * fix(agent-launch): combine both launch warnings instead of dropping one Round 2 found the comment here was false. A create warning and a surface warning CAN both be set, on two reachable paths: 1. The create warns precisely BECAUSE it produced no startup terminal - didSpawnStartup stays false when that spawn throws, and orca-runtime-create-managed-worktree.ts:283 gates startupTerminal on it - so the executor's early return is skipped and a second surface is built, which can warn too. 2. An untracked-copy warning, then a definitive structured refusal downgrading to a terminal that also warns. `??` kept the first and lost the second with nothing saying so. They are now combined the way the create combines its own failures - appendFailure in runtime-local-worktree-terminal-startup.ts, and the startup-terminal catch in runtime-remote-managed-worktree-create.ts - which append rather than replace. The comment is rewritten to say what is true, and records the gap NOT fixed here: a create warning about a failed startup terminal is stale once the launch recovers by building a working one, so a user can be told the agent did not start while looking at it. Distinguishing those needs createManagedWorktree to stop multiplexing two unrelated failures into one string. Guarded and ablated: restoring `??` reddens exactly the new test, with the surface clause missing from the received string; restored to 27 passed. The structured-create stub had to admit its real ok-or-refusal union for the downgrade path to be modellable at all - it previously declared only the ok arm. Also: mobile.yml gains src/shared/agent-launch-intent.ts. It is the sole holder of the agent.launch RESULT shape - the rpc-contract catalog holds params only - and mobile imports it as a value. CROSS_VERSION_WIRE_PREFIXES already treats it as wire-critical; without this, one gate does and the other cannot see it. And the agent-first warning test no longer pairs "startup terminal failed" with a returned handle, a combination the producer cannot emit. * fix(mobile): read a launch warning an older host nests on the outcome agent.launch moved `warning` from the terminal outcome to the top level of the result. That is the right shape - a reader should not branch on `outcome.kind` to learn the workspace it just opened is incomplete - but on the wire it is a REMOVAL, and mobile only read the new place. A host built before the move still advertises the same `agent.launch.v1` capability, so the capability probe cannot tell the two apart and mobile takes this route against one: protocol-version.ts:360 AGENT_LAUNCH_RUNTIME_CAPABILITY is in RUNTIME_CAPABILITIES, the host list orca-runtime-get-status.ts:64 publishes it via status.get; the filter drops only browser.screencast.v1 and three E2E-gated capabilities, never agent.launch agent-launch-executor.ts such a host writes warning INSIDE outcome The result was a regression rather than a contract cleanup: the worktree.create path this replaces returned the warning at the top level and mobile read it, so a create that seated the workspace but could not start the agent surface - pty exhaustion, untracked files not copied - stopped explaining itself on the phone. Read both shapes for as long as such a host can be paired. Top level wins, and cannot be shadowed: AgentLaunchOutcome has no `warning` on either arm, so a current host cannot nest one. The test that pinned the old behaviour is inverted here. Its comment was the actual defect - it framed a legitimate warning from an older peer as a stale shape to defend against, which is what made dropping it look deliberate. * chore(mobile): raise the unchecked-reader ceiling for the agent.launch receipt main landed `unchecked-rpc-reader-inventory.ts`, a ratchet on RpcOperation readers that re-type their reply instead of validating it. Its ceiling for mobile-workspace-create-operations.ts is 4, counted on a tree without this branch's `agentLaunchRun`, so the merge produced "listed 4, found 5". The inventory's own header prescribes this case: a merge is the one time a line goes up without a migration undoing itself, and the instruction is to raise it and name the PR that brought it. It describes main landing an operation the branch never saw; here it is the mirror - the branch holds one main had not seen - so the line is annotated with #19850 rather than left bare. Not converted to `rpcResultVariant(variant, schema)`, which would lower the line instead. That is a validation change rather than a migration, which is exactly what the file's own comment says these five readers deliberately are not; the agent.launch reply is already guarded at the consumer, where readAgentLaunchCreateOutcome returns null on a malformed payload and the create surfaces "Failed to create workspace". Writing a schema now would also target a reply shape #20999 is actively redefining. Ablated: with the line back at 4 the ratchet fails "listed 4, found 5"; at 5 it passes. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-16 13:15:20 -07:00
Jinwoo Hong	ea7902cbee	refactor(mobile): send the device-state holdouts through typed RpcOperations (step 4, wave 3) (#20915) ... * test(mobile): record the terminal input surface before migrating it (step 4) Three families the recorder could not reach before, recorded against the pinned baseline's product code so the migration that follows has a parity oracle. The device state these hooks read is real, not declared. The pasteboard is the engine's existing per-recording fixture, so a paste reads the bytes a recorded copy put there one action earlier; the buffered draft store is the product's own useBufferedTerminalDrafts mounted in the same tree. No engine file is touched, so no existing golden moves and no header re-digests: 13 new goldens, 641 unchanged. Only the clipboard's text path is driven. The image path decodes a raster through expo-image-manipulator and stages it on expo-file-system, and recording it would mean inventing image and file-system behaviour. Both paths reach the same send. Two family mutants, one per family whose state() can observe a reply: keeping a refused send's draft cleared, and resolving the first repo's connection instead of the workspace's own. The paste family gets none — the hook returns void and calls onSuccess for an accepted and a refused send alike, so its only reply-dependent behaviour is the takeover report, which lives in the sender list. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the device-state holdouts through typed RpcOperations (step 4) Ten references over six files, the last of the raw-port sites whose blocker was that a recording could not reach them. Zero goldens move: every one of the six was recorded first, and the suite replays them against the rewritten code. Two operations are new and four sites reuse one that already fixes their method: - accounts.consumeCodexResetCredit, throw-message, payload unread — the call site's decodeResetResult is one scope-and-snapshot check and splitting it across a reader would put one refusal rule in two places. - notifications.getMissedSince, skip — a background pass with no screen to raise a host message on. The member read stays where the optional chaining was. - repo.list: the accessory's connection lookup joins the new-tab reader, which already threw the host's message; the new-workspace dialog joins the skip reader, which already left the list it had. Same reader, same policies, no new acceptance rule and no third operation on that method. - terminal.send: the composed send, the live keystroke send and the clipboard paste all join terminal.input-send, which the accessory raw send already used and which reads acceptance the same way isTerminalSendRpcAccepted did. The typed contract is stricter than the client's own scope type on the redeem: the catalog pairs each runtime with the distro it may name, while the shared CodexResetCreditExpectedScope does not. The invariant is real and held by the attempt journal's schema, so the narrowing is asserted at the send with that named; the bytes are unchanged. Widening the catalog would be a wire change. Two source-shape ratchets pinned the old call text and move with it. The route parity suite's runtime strings drop from 540 to 537: the three method literals that became operation definitions, and nothing else. Every hook, callback identity, effect, JSX and style pin is unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct the terminalInputSend and PTY-mode holdout comments `terminalInputSend`'s doc still claimed two call sites. It now has five non-test consumers, all on the same acceptance: the query-reply responder, the live accessory raw send, the session screen's composed draft send and live keystroke send, and the clipboard paste. That comment is where the next person narrowing `object-result-or-null` learns whose lost-ack meaning they are changing, so it names all five and their files. The session inventory block closed with "opens or rides a subscription, or takes its method as a parameter", which no longer covers every holdout below it: `use-mobile-session-terminal-input.ts` is held out for a webview handle. Its own reason also said PTY mode was unavailable in the runner, which this branch's terminal-input adapter contradicts by fixturing the mode map a paste reads. The sentence is narrowed and the holdout restated: PTY mode is recordable, the live webview handle is what is left. Comments only. No product behaviour, no golden, no parity hash moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pair the draft-restore mutant with the refused send `terminal-send-refusal-restores-draft` documents the harm of a refused send that leaves the composed draft cleared, but it was driven by the accepted scenario, where the kill comes from the inverse (a draft restored after a send that landed). The refused scenario shows the documented harm directly: without the restore the input stays empty after the runtime says no. Still one mutant per family, and it kills there — verified by running the suite, `terminal-input-send-refused: kills terminal-send-refusal-restores- draft`. No golden, no product change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): correct the gesture-input holdout and drop a dead repo type Three round-2 corrections, comments and one dead type; no behaviour. The gesture-input holdout claimed a recorder gap that does not exist. The flush path reads refs only — client, connection state, PTY modes, the gesture buckets, active handle and tab type — and the clear-buffer reference optional-chains the webview ref, so a mount with a null terminal ref puts both sends on the wire. The reason now says what is true: those 2 references are migratable as they stand and were out of this PR's bucket. The session summary sentence no longer offers a webview reason. `RuntimeRepoSummary` in mobile-session-route-types.ts lost its last consumer when the accessory hook moved to `MobileRuntimeRepoSummary`; `git grep RuntimeRepoSummary` now finds only the `Mobile`-prefixed type. Deleted. Both refreshed route-parity hashes still credited the `interpretOrThrowRefusalMessage` refresh for their current value. They now state the invariant they pin and this PR's reason for the move: the sends and repo reads inside those bodies name their `RpcOperation` instead of the raw `sendRequest` port. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-15 21:12:33 -04:00
Jinwoo Hong	36ef93a64f	refactor(mobile): migrate the small domains onto RpcOperation (step 4) (#20705) ... * refactor(mobile): pin each RPC golden to its own mount adapter, not every domain's `recorderSha256` covered the whole recorder directory, mount adapters included, so a domain PR that adds its adapter module moved the header of all 153 goldens. #20568 did exactly that and its merge with main conflicted on that one line in 153 files; every future domain PR would collide with every other in flight the same way. Split the directory at a real seam instead of a filename convention: `adapters/` holds one module per domain, registered in `adapters/mounted-operation-modules.ts`, and `recorderSha256` now covers the engine only. A new `adapterSha256` covers the source of the module that mounts each operation a golden's scenarios drive, read off the same `mounts` calls that build the table the recording runs against, so the pin cannot name a file the runner did not use. Adding a domain's module now re-digests nothing already recorded; editing one fails exactly the goldens mounted through it. `adapter-seam.test.ts` keeps the split from drifting: an engine file inside `adapters/`, an adapter defined in an engine file, a register entry naming the wrong file, and an adapter importing a sibling each fail. The five adapters that were inline in `pilot-mount-adapters.ts` move into their own modules, which leaves that file as the registry and nothing else. `GOLDEN_FORMAT_VERSION` goes to 5 for the new header field; the goldens re-record in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the split recorder/adapter digest Header-only. Every changed line is `recorderSha256` (the engine digest no longer covers `adapters/`), the new `adapterSha256`, or `goldenFormatVersion` 4 -> 5; `baseline` is unchanged and recording ran against the same pinned product tree. git diff -U0 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256|goldenFormatVersion)":' | wc -l 0 The seven `adapterSha256` values partition the 153 goldens by the module each was recorded through: 58 settings, 37 hosted review, 21 source control, 11 new-tab agents, 9 file inventory, 9 tasks, 8 workspace settings. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): stop pinning goldens to recorder inputs no recording can read The adapter split left three per-domain edits still moving all 153 headers: the mutant table, the per-family mutant registry beside it, and the probe-hole witness. None can change a recording -- the loader consults a mutant only when a mutant test asks for one, and no suite but the two recording drivers writes a golden -- so pinning them claimed a provenance the goldens do not have and charged every domain a full re-record for it. `mutants/` now holds the table, the registry, the reference states, the mutant suites and the probe-hole witness, and `recorderSha256` skips it. What makes that sound is that no recording can reach it: `operationModuleLoader` takes a resolved mutation spec instead of importing a table by name, so nothing on the recording path names `mutants/` at all. `mutants/mutant-seam.test.ts` checks exactly that, and fails if an engine file names the directory or anything outside imports from it. `recorderSha256` also pins only the suites in `recording-drivers.ts`, which `scripts/rpc-recording.mts` records from, so the two cannot drift. A suite that reads goldens, or writes one to a scratch directory, is no longer provenance for a recorded file. `OPERATION_EXPOSURES` went the other way, because it does change what a recording loads: withhold the resume-metadata exposure and exactly four goldens fail. Each domain module now declares its own exposures and gets its own loader, so `adapterSha256` pins the ones that reached each golden. Two assertions in the digest boundary test were vacuous: `join(root, '.')` normalises back to `root` and hit `recorderSha256`'s per-root cache, so the prose-is-ignored claim never recomputed anything. Each call now spells the root differently. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens under the mutant and driver exclusions Header-only, and no format bump: the header shape is unchanged. `recorderSha256` moves on all 153 because the engine set shrank, and `adapterSha256` moves on the 58 settings goldens because that module now carries its own exposure declaration. git diff -U0 HEAD~1 -- mobile/rpc-foundation/goldens | grep -E '^[+-]' \ | grep -vE '^(\+\+\+|---)' \ | grep -vE '^[+-] "(recorderSha256|adapterSha256)":' | wc -l 0 Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): restore the preferences actions the merge resolution dropped #20568 added `resume` and `trust` actions to the `settings.task-preferences` adapter while it still lived in `pilot-mount-adapters.ts`. This branch had already moved that adapter into `adapters/task-mount-adapters.ts`, so resolving the `pilot-mount-adapters.ts` conflict in favour of the registry merge silently discarded them and `tw-task-preferences-resume-write` failed to record at all ("Missing or completed request: ui.set#1"). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens at main's tip after the merge All 208 goldens, header-only. `baseline` moves from 50e752fc66 to main's tip c6a7216984, `goldenFormatVersion` from 4 to 5, `recorderSha256` to the value of the engine with `adapters/` and `mutants/` carved out, and `adapterSha256` is new on every file. Nine distinct adapter digests over 208 goldens: each golden now pins only the module that mounts it. No observation moved. The whole-diff census against origin/main reports exactly four changed keys and nothing else: 208 "adapterSha256": 416 "baseline": 416 "goldenFormatVersion": 416 "recorderSha256": Recorded in place rather than through the README's detached-baseline dance: this branch changes no product file, so its tree at the merge is byte-identical to c6a7216984 under mobile/src, src/shared and the lockfile, and the parity claim stays non-circular. README says so now. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): hold the recording drivers to the engine's mutant-seam rule The name scan exempted every `.test.ts` on the ground that a test cannot change a recording. Two of them can: the recording drivers are the recording path. A driver that read the mutant table by path rather than importing it passed both seam checks — the import scan sees no import, and the name scan waved it through as a test: const table = resolve(import.meta.dirname, 'mutants/operation-mutations.ts') console.log(readFileSync(table, 'utf8').length) at the top of `pilot-recordings.test.ts` gave 2 passed before, and after this change fails with ["pilot-recordings.test.ts"]. Only non-driver tests are exempt now. This file lives in `mutants/`, which `recorderSha256` skips, so no golden moves: the recorder suite is green on the existing 208 with zero dirty. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the registry parameter no caller varies `pilotMountAdapters` took `registered` so a caller could mount a different module set; all six callers take the default. The header-digest tests vary the registry through `goldenRecording`, which keeps its own parameter and is where the stub roots need it. Engine source, so `recorderSha256` moves and the goldens follow in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens after the registry parameter came out All 208, `recorderSha256` only. The re-record against the previous commit moves 416 lines, every one of them that field: 416 "recorderSha256": Against origin/main the picture is unchanged from the merge: 208 goldens, 0 added or deleted, 0 non-header lines, and exactly four keys differing — 208 "adapterSha256" 416 "baseline" 416 "goldenFormatVersion" 416 "recorderSha256" Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): wrap the recording README at the width the rest of it uses Seven lines this branch added ran past 100 columns, worst 124. No wording changed. Markdown is outside `recorderSha256`, so no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): name the worktree overlay, not the archive that cannot work `git archive` was offered alongside a detached checkout as a way to lay this branch's recorder over the pinned baseline. It cannot work: the fence in scripts/rpc-recording.mts runs `git diff --quiet <baseline>` and an untracked-file check, both of which need a real `.git`. In an archive tree git exits non-zero for lack of a repository and the script reports "Product sources or lockfile differ from the pinned main baseline", which reads as a product mismatch that is not there. The transport agent lost time to exactly that. Names `git worktree add --detach` only, and says what the misleading failure looks like if someone tries an archive anyway. Markdown is outside `recorderSha256`, so no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): close two ways an adapter module escapes its own digest Two holes, one class: the seam was checked by how an import was spelled and by what the register's values evaluated to, never by where they resolve or where they were written. Inward imports: the scan dropped every specifier starting with `..`, so `'../adapters/settings-mount-adapters'` climbed out of the directory and back into it unseen. A reviewer had `new-tab-agent-mount-adapters.ts` project a value read from the settings module, edited that module, and watched the mounted state change while the new-tab adapter digest held. Specifiers now resolve against the directory and anything landing back inside it fails: ["new-tab-agent-mount-adapters.ts imports ../adapters/settings-mount-adapters"] The register: `adapters/mounted-operation-modules.ts` is pinned by nothing — `recorderSha256` skips the directory and `adapterSha256` reads each entry's `source`. An `exposes` written inline there drives the mounted product module with no digest covering it. The same reviewer replaced the new-tab entry's `exposes` with a literal overriding `loadMobileNewTabAgentOptions`; twelve fence tests passed. Both `mounts` and `exposes` must now be identifiers the register imports from that entry's own module: ["new-tab-agent-mount-adapters.ts writes exposes inline instead of importing it"] Checked on the register's syntax, not its values, because an inline literal and an imported binding are indistinguishable once evaluated. Pinning the register in the engine digest would also close it, and is the wrong trade: every domain adding a register line would re-digest all 208 goldens, which is the conflict this PR exists to remove. Keeping the register an index costs nothing and keeps a domain's line local. Both fixes live in a `.test.ts` outside the drivers, so no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): prove the mutant seam from the drivers out, not by spelling The seam rested on a grep for the literal `mutants`, which the exported `MUTANT_DIRECTORY` spells without containing. A reviewer had `pilot-mount-adapters.ts` read the mutant table through that constant and both checks passed. The README's claim — that nothing on the recording path names the directory — was false as written. Three changes, in order of strength: Reachability is now proved forward. The suite walks the static import graph from the two recording drivers and fails if any module under `mutants/` is in it. That answers the real question, what a golden's bytes can depend on, instead of the old inward scan's question, who mentions this directory. Non-emptiness is asserted on both sides so a graph that resolved nothing cannot pass by reaching nothing. The name scan covers both spellings, for paths a module can be read by rather than imported. The reviewer's probe now fails as ["pilot-mount-adapters.ts"]. `MUTANT_DIRECTORY` is no longer exported. Its two consumers were both tests of the digest, and they now spell the path instead, which is strictly better for them: a test that imports the constant follows a rename silently, while one that spells it fails on a rename — and that specific directory name is the whole soundness argument. This edits `recorder-digest.ts`, so the goldens re-record in the next commit. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the RPC goldens after MUTANT_DIRECTORY stopped being exported All 208, `recorderSha256` only. Against the previous commit the diff is 416 lines and every one of them is that field: 416 "recorderSha256": Against origin/main, unchanged: 208 goldens, 0 added or deleted, 0 non-header lines, four keys differing — 208 "adapterSha256" 416 "baseline" 416 "goldenFormatVersion" 416 "recorderSha256" Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state the mutant seam's actual argument, and its edge The README claimed nothing on the recording path names `mutants/`. That was the old inward scan's claim and a reviewer falsified it with the exported constant. It now describes what the check does: a forward walk of the import graph from the two recording drivers, plus a name scan in both spellings for read-by-path, plus the constant no longer being exported. It also names the case neither closes — a path assembled from fragments at runtime. Markdown is outside `recorderSha256`, so no golden moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): prove the engine/adapter seam in both directions The inward scan only held adapters to the seam. An engine file importing an adapter executes code its own digest skips and that every golden recorded through another domain leaves out of `adapterSha256`, so the register is now the only crossing allowed from the engine side. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): name what the driver walk missed instead of counting it Seeding `seen` with the drivers made the driver-presence check true by construction, and the size bound compared a graph inflated by `typeof import` product modules against a recorder-sized number. Both go; the walk now reports the recording files it failed to reach, which is empty today and names an orphan engine file the moment one appears. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): reflow four paragraphs left ragged by the rewrap Orphan fragments only, no wording change: the golden-schema field list, the mutant-evidence paragraph, the probe-witness sentence and the re-anchor note. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the small-domain call sites before migrating them Thirteen new families cover the files, new-workspace, host-screen, home and worktree call sites step 4 migrates next: ownership capture, the preview loader and its terminal-artifact grant refresh, the artifact save, the tab doc's three shapes, the drawer's execution target and setup hook, the Codex reset-credit probe, the host view settings, the Home stats card and the three workspace catalog reads. Recorded against main's product code, so these are the parity baseline the refactor must not move. Four new adapter modules under adapters/ and no engine edit, so recorderSha256 is unmoved and every existing golden is byte-identical: 40 files added, none changed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the small-domain reads through RpcOperation Thirty-five of the domain's fifty-five raw-port references now go through a declared operation: the files domain's preview, artifact and tab-doc reads and its ownership capture, the New Workspace drawer, the host screen's metadata and view-settings mirror, the Home stats card, and all three workspace catalog reads. No behaviour change, and the oracle says so: zero goldens move. Acceptance is preserved call site by call site, including two that look like defects and stay that way — a refused worktree.listRetiredNames still settles as an empty registry rather than holding the previous names, and a refused ui.get on a null result still throws into the host screen's own catch. Where two call sites disagreed about one method, both policies are named: files.read and files.readPreview throw for a session file tab and skip for the preview screen, repo.hooks throws for task create and skips for the drawer, and status.get now carries a fourth family for the Codex capability probe's object-or-null rule. The drawer's SSH connect, SSH state and agent detection reuse the workspace-create operations the tasks migration already declared rather than restating them. Two things outside the call sites. requestSingleFlight now shares the params optionality rule that request already had, so an all-optional schema such as preflight.check can omit its params on both helpers instead of only one; that is type-level and puts nothing new on the wire. And the retired-names fixture resolved a reply with no `ok`, a shape no host sends, which read as a refusal once the acceptance policy routed on it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): drive the image arm of the preview loader The mutation census found two migrated operations that no recording reached: repointing files.readPreview or files.readTerminalArtifactPreview to a wrong method, and swapping either one's acceptance policy, changed nothing any golden observed. Both preview-load scenarios read a text path, so the loader's image branch was migrated with no wire behind it. Two scenarios now read an image path through each arm, and the adapter takes the path from the scenario instead of a constant. All four mutations die on the new goldens. They are recorded from the pinned baseline with this branch's recorder laid over it, so they are main's behaviour and not the migration's: the candidate run against the refactored tree compares clean. The adapter edit re-digests the nineteen goldens mounted through it. The diff is one adapterSha256 line each and no observation moves, which is what pinning the adapter per golden rather than per suite is for. Two casts also take the SAFETY form the house style asks for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the casts the changed-code gate flags Seven type assertions the gate counted as new, all removed rather than silenced where the type system could already answer. `normalizeMobileFilePreviewResponse` narrows on `ok` instead of asserting each arm; the snapshot adapter narrows on the fetch result's own discriminant; and the drawer's two probe reads go through one overloaded member read that keeps their optional-chaining behaviour. The remaining three keep a cast and now carry the rationale on the asserting line. No behaviour change. The two adapter edits re-digest the sixteen goldens mounted through them, one adapterSha256 line each with no observation moved, recorded from the pinned baseline the same way. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the import the narrowing left behind RpcSuccess is no longer named once the response reads through its own discriminant. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): record the host screen's worktree mutations Review of #20705 showed the `use-host-worktree-actions.ts` holdout reason was wrong: its only native call is the pinned-id write, and that sits behind `if (hostId)`, so mounting with no hostId never reaches it. Two scenarios in one new family, recorded from the pinned baseline with the call site still on the raw port. The first drives all three sends so the reply matrix covers each method; the second refuses `worktree.rm` to pin the row restore. The adapter is a new module, so no existing golden's `adapterSha256` moves and none of the 250 goldens already here is rewritten. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): send the host list's worktree mutations through RpcOperation Pin, remove and activate move onto operations in host-screen-operations.ts. All three skip on refusal, which is the policy each site already applied by hand: the pin and activate writes swallow everything in a `.catch`, and the remove restores the row on a refused reply. `worktree.set` therefore carries a second policy next to source-control's `worktree.set-review-link`, which throws; both are named. Zero goldens move. The inventory loses use-host-worktree-actions.ts and states the real reason the drawer's repo list stays: it renders the last-visited-repo hook, whose default import of async-storage the recorder's proxy refuses at module load, before the hostId guard the reviewer expected to save it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): type the worktree-action fixture row and follow the activation scan The fixture row I recorded from had four fields, which `tsc` rejects as a `Worktree`. Filling it out moves the five goldens of this branch's own new family and nothing else; the recorded sends are unchanged, only the projected row is. `mobile-worktree-activation-source.test.ts` scanned the hook for the literal `sendRequest('worktree.activate'`, which the previous commit replaced. It now asserts the operation call and its two flags in the hook, plus the method in host-screen-operations.ts, so the pair still pins the same wire. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): take the five deletions round-1 review asked for - `fileOwnershipRuntimeStatusRead` was `taskRuntimeStatusRead` field for field. It is now a re-export of it. The goldens are keyed on the scenario family, not the operation name, so `matrix-files.mutation-ownership-status.get-1.json` survives unchanged. - `readProbeMember`'s two overloads asserted shapes nothing checked. Gone; the nested read goes through the same single-signature function. - `normalizeMobileFilePreviewResponse` had no product caller. Deleted with its re-export; its twelve assertions now drive the accepted and refused arms directly. - The three inline copies of the accepted-result union are gone. They name each operation's own `interpret` return instead of importing `RpcAcceptedResult`: importing the contract would pull all three call sites into the cast fence, where their existing SAFETY assertions fail it. - `codex-reset-credit-capability-operation.ts` is now `-operations.ts`. No adapter names it, so no golden re-digests. Zero goldens move. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): give the skip verdict its own transport module The three settle helpers typed their interpret parameter as `ReturnType<typeof <op>.interpret>`, naming one operation while being called with others whose verdicts happen to be structurally identical. Narrowing a named reader would have silently retyped unrelated helpers. `RpcAcceptedResult` moves to `rpc-accepted-result.ts`, outside the cast fence's three region seeds, so a consumer can name the verdict without becoming an operation implementation. `rpc-operation-contract.ts` re-exports it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop three sender aliases nothing imports MobileHostScreenRpcSender, MobileNewWorkspaceRpcSender and MobileWorktreeCatalogRpcSender each appeared only in the file that declared them. A named type with no consumer is a cost, not a boundary. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say why a holdout is a holdout in the port inventory A site can be pointed at an operation without being mountable, so "cannot be migrated" was the wrong claim. The rule is record-first: the golden recorded against the old code is the only parity proof, so a site the recorder cannot mount cannot be recorded, and unrecorded sites do not migrate. Stated once in the list's header. codex-reset-credit.ts loads fine under the module loader; probed it, and its attempt-journal access throws on async-storage at call time before the send, with no guard to skip it. The old comment described it as a storage read around the send. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state the status.get policies without counting them "the fourth policy on this method", "the first of two" and "the second of two" were already wrong after round 1 folded the files family's status read into the tasks one. Each comment now states its own invariant, which no later policy can invalidate. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): stop the activation scan claiming to pin the wire `expect(operations).toContain("method: 'worktree.activate'")` passes whichever operation carries that method, so it survives swapping worktreePinWrite's and worktreeActivate's methods. tsc and the host-worktree-actions-pin-open-delete golden both fail on that swap; the scan keeps only what it can prove, which is that the callback sends through worktreeActivate with the two flags. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-15 13:10:32 -04:00
Neil	231e805b1e	fix(lint): enable anti-slop/no-shape-in-symbol-names (#20785) ... Flip `anti-slop/no-shape-in-symbol-names` from "off" to "error" and clear every violation under src, config, tests and mobile. What the rule bans ------------------ The case-insensitive substring "shape" in any JS/TS identifier: variables, functions, parameters, types, type parameters, class members, private names, object-literal keys and JSX identifiers. The one exemption is a statically accessed member read owned by another value (`zodObject.shape` is fine), so third-party APIs stay readable without a suppression. "Shape" names a value's structure rather than its domain role. `UserShape`, `validateArgShape` and `errorShape` all tell you the symbol is "an object with some fields" -- which is already what a type says -- while saying nothing about what the value is for or who owns it. The rule forces the name to carry the domain instead. Violations fixed ---------------- 689 violations across 109 files at baseline (verified by re-running the audit against the pre-change tree with the rule set to "error"). Fix pattern ----------- Rename for the domain role, not the structure: -type FieldShape = 'list' | 'map' | 'whole' -const FIELD_SHAPES = { ... } satisfies Record<keyof Observation, FieldShape> +type FieldEncoding = 'list' | 'map' | 'whole' +const FIELD_ENCODINGS = { ... } satisfies Record<keyof Observation, FieldEncoding> -function assertGitPushTargetShape(target: unknown): void +function assertValidGitPushTarget(target: unknown): void -function describeReadDirPathShape(p: string): ReadDirPathKind +function classifyReadDirPath(p: string): ReadDirPathKind Predicates became statements about the value (`isDeltaShapedProviderFrameKind` -> `isDeltaProviderFrameKind`, `isDeleteShapedDiscardEntry` -> `discardDeletesEntryFile`, `isSkillsCliAgentKeyShaped` -> `isUsableSkillsCliAgentKey`). Type aliases dropped the suffix where the remaining name was already unambiguous (`GhGraphqlErrorShape` -> `GhGraphqlError`). No wire-visible name was renamed: no IPC or RPC channel, stream opcode, request/response param, persisted field, or i18n key. The `--shape=symlink|copy` CLI flag read by .github/workflows/skill-update-roundtrip.yml is unchanged -- only the local variable holding it was renamed. Exemptions ---------- They are file-scoped entries in config/oxlint-anti-slop.json, not inline `oxlint-disable` comments. An inline directive naming an anti-slop rule reads back as an UNUSED directive under the root lint scan, which does not load this plugin -- the changed-code quality gate counts that warning, so the comment form cannot be used for a rule that lives only in this config. * src/renderer/src/components/browser-pane/annotate/**: in the screenshot annotator a "shape" is the drawn geometry -- pen, arrow, rect, ellipse, highlight. That is a genuine domain noun, and it pervades every symbol in the module. * repo-icon.tsx, repo-header-project-actions.tsx, mobile MobileRepoIcon.tsx: lucide exports the icon component as `Shapes`. The name is theirs, and the matching REPO_LUCIDE_ICONS key is the persisted icon name shared with the desktop picker -- renaming it would orphan saved repo icons. * src/shared/onboarding-state-types.ts, src/shared/constants.ts: `shapedSidebar` is a persisted onboarding-checklist field and a telemetry enum member; renaming it would orphan saved state. * src/shared/rpc-contract/rpc-send-params.ts: matching zod's own literal `shape` property is what selects the ZodObject branch of the conditional type. No exemption was added merely to avoid a rename. Eight symbols initially suppressed as "a cross-module refactor outside this change" were proven to have zero non-TypeScript references repo-wide and renamed instead. Zod's `ZodRawShape` needed no exemption at all: `Readonly<Record<string, z.ZodType>>` is its definition, so repo-update-params.ts and ui-update-value-tolerance-params.ts spell it out instead. Likewise telemetry-event-classification.ts now reads `.shape` through an `in` narrowing, which also retires two pre-existing type assertions; three more assertions the rename had dragged onto changed lines (two `JSON.parse` sites, one node:sqlite row read) became annotations and an explicit row mapping. Verified -------- * Audit reports zero violations; confirmed the rule genuinely fires by planting a probe violation. * node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0. * Vitest over src/shared, src/main/github/project-view, the annotate module, the repo-icon components and the Chromium SameSite electron spec: all green. * All 66 removed "shape" identifiers grepped repo-wide across every file type; none survive. * node config/scripts/generate-rpc-params-catalog.mjs --check exits 0. * node --check on every changed .mjs; oxfmt clean on all changed files. * `pnpm run check:code-quality:changed` reports 0 findings. Not machine-verified: the 3 mobile/ files (its Vitest run cannot resolve `expo/tsconfig.base.json` in this worktree), and the WSL- and Playwright-gated specs. All are rename- or comment-only hunks, read in full.	2026-09-15 02:00:27 -07:00
Jinwoo Hong	e53f1557e1	fix(mobile): two known main bugs the RPC migration preserved (#20563) ... * fix(mobile): two known main bugs the RPC migration preserved A malformed host `error` and a null settings result both reach a property read that throws. Both are deliberate behaviour changes; the goldens move in the follow-up commit. `hostReplyErrorTextOrFallback` passed a truthy non-string through under a `string` annotation. Its one caller is the in-band `git.commit` failure, and every consumer of that text is display or prompt copy: `use-mobile-create-pr-runner` and `PrSidebarCreateEmptyState` record it as a commit failure, `use-mobile-commit-failure-recovery` hands it to `summarizeCommitFailure`, which starts with `raw.slice(...).replace(...)`. So no consumer needs the value, and the decision is the fallback rather than `String(value)` — the relay handler declares `commit(): Promise<{ success: boolean; error?: string }>`, so a non-string is a malformed reply, and `generatedCommitMessageReader` in the same domain already reads a non-string host error as absent. The parameter stays `unknown`, which it honestly is, and the `SAFETY` cast is gone. `useNewWorkspaceRuntimeContext` read settings through `settingsRead`, whose reader preserves main's `boxed!.settings` throw, so a `null` or absent result threw a TypeError out of the effect — losing the trusted-hooks publish and the available-provider computation that follow it, not just the settings. It now uses `optionalSettingsRead`, the operation that already reads a null or absent result as absent settings, so the reply degrades exactly the way a reply with no `settings` member does. Reply-side only: same method, same params, same barrier, no wire change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the goldens the two bug fixes move Baseline bumped to 3f71999237. Two goldens move an observation; the other 151 move only `baseline` and `recorderSha256`, which `pilot-scenarios.json` is still digested into. Observation moves, one claim each: - `matrix-hostedreview.create-intent-git.commit-1`, partition `inner-false-object-error`: `settlements.run.value.error` and `state.outcome.error` go from `{"message":"inner refused"}` to `"Commit failed"`. A non-string in-band `git.commit` error is a malformed reply and now reads as the screen's copy, converging with `result-absent`, `result-null` and `outer-refused-no-message`, which already reported the fallback. The other ten partitions at this site are unchanged. - `matrix-settings.workspace-context-settings.get-1`, partitions `result-null` and `result-absent`: the `unhandled-rejection` TypeError effect (`reading 'settings'`) is gone and `state.providers` goes from `[]` to `["github"]`. The effect no longer aborts the rest of the hook, so the provider computation runs; `state.settings` stays null because nothing was published, which is how a reply with no `settings` member already degraded. The other nine partitions are unchanged. Header-only moves: - 9 goldens of the `settings.workspace-context` family rename `namedDeltas` from `new-workspace-runtime-context-null-settings-typeerror` — the name now lies, the TypeError is fixed — to `new-workspace-runtime-context-null-settings-degrades-to-absent`. - All 153 move `baseline` and `recorderSha256`. The digest covers `pilot-scenarios.json`, so the baseline bump and the rename re-digest every file. No sender recording moved: both fixes are reply-side, and no golden's `sender` or `payloads` field differs. The README paragraph that claimed the settings TypeError was preserved is updated, and now records that the `ui.get` leg of the same hook still is. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): degrade a null ui.get result the way the settings leg now does One host answers both legs of useNewWorkspaceRuntimeContext, so fixing only settings.get left the likelier failure in place: a null or absent ui.get result still threw `reading 'ui'` out of the effect, skipping the provider commit. Review follow-ups on the same files: reply() returns the literal uncast and the stub client is FakeSession, dropping two assertions and their SAFETY disables; the degradation cases now assert absolute state instead of comparing mounts. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-record the goldens the ui.get leg fix moves Observational move, 1 golden: - matrix-settings.workspace-context-ui.get-1: the `result-absent` and `result-null` partitions drop their `reading 'ui'` unhandled-rejection effect and their state commits `providers: ["github"]` instead of `[]`, because the effect no longer throws before the provider commit. Header-only moves, 153 goldens: `baseline` to the fix commit and `recorderSha256`, which covers `pilot-scenarios.json` and so re-digests on the delta rename. The delta is renamed `new-workspace-runtime-context-null-settings-degrades-to-absent` -> `new-workspace-runtime-context-null-results-degrade-to-absent` (9 goldens): it now covers both reads, not just settings. README updated to match. No sender recording moved: resolving the value pool across all 153 goldens shows `sender` and `payloads` byte-identical everywhere. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): name the ui.get result shape so the changed cast carries a rationale The inline union wrapped over four lines and tripped the changed-code casting gate as a new assertion; a named alias keeps the cast on one line under a SAFETY note. No behaviour change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the golden baseline to the cast-rationale commit Header-only: `baseline` on all 153 goldens. The re-record is inert — no golden moves observationally and no field other than `baseline` changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the ui.get trust blank, and correct two stale acceptance comments The goldens cannot catch a regression to `if (uiResult?.result)`: the scenario's success reply is `{"ui":{}}`, so every partition of matrix-settings.workspace-context-ui.get-1 records the same `trust:{}` state. The new case answers once with real trust and again with a null result on a fresh client, which is the only shape where skipping the blank is observable — trustedOrcaHooks gates the setup-hook approval prompt in use-new-workspace-create-submit.ts, so a stale value would skip it. settingsRead's comment still claimed workspace context, which this branch moved to optionalSettingsRead; both comments now name their real callers. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): repin the golden baseline to the trust-blank commit The record fence rejected the previous pin ("Product sources or lockfile differ from the pinned main baseline"), so the branch was no longer re-recordable. Header-only: `baseline` and `recorderSha256` on all 153 goldens — the digest covers pilot-scenarios.json, whose only edit is that baseline. The re-record is inert: 0 goldens move observationally and no other field changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): name the right operation per refuse-after-data probe Three of the five probes read through optionalSettingsRead, not settingsRead: repo metadata and resume metadata already did, and workspace context does as of this branch. The sentence now splits them and states why the split does not move what the probes record. Markdown is excluded from recorderSha256, so no re-record. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the recorder's unhandled-rejection capture This branch removed the last two goldens that recorded an unhandled-rejection effect, so nothing exercised unhandled-recording.ts any more: gutting the emit to `void captureError(error)` leaves all 153 goldens comparing clean. The unit test drives a detached rejection through the window and asserts both the effect and the listener restore. README says so where it describes the capture. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): re-digest the goldens for the new recorder test Header-only: `recorderSha256` on all 153 goldens, which covers every non-markdown file under rpc-recording/ and so moves for the added test file. The re-record is inert: 0 goldens move observationally and no other field changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-14 11:48:28 -04:00
Jinwoo Hong	b0070e3720	refactor(mobile): migrate settings reads to RpcOperation (#20499) ... * refactor(mobile): migrate settings reads to RpcOperation Replay the settings slice on the landed RPC foundation after rebasing onto main. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh task parity snapshots after main rebase Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): correct rebased declaration parity hash Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): account for main task declaration Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): preserve raw RPC rejection timing Return the transport promise directly and interpret replies separately so sibling Promise.all rejection order cannot change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): refresh parity hashes after timing fix Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): use operation interpreter after raw request * test(mobile): refresh settings migration parity hashes Refresh hook and statement parity hashes for the two task declarations whose settings reads now use RpcOperation request and interpretation. Changed declarations: - useMobileTasksRuntimeHydration: settings.get replaced by settingsRead request/interpret. - useMobileTasksWorkspaceCreateActions: settings.get response handling replaced by settingsRead request/interpret. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-13 17:53:58 -04:00
Neil	fbab61ec09	perf: skip unclosed suffixes when stripping review markdown tags (#20329)	2026-09-12 18:19:16 -07:00
Neil	1b5092492f	perf: scan mobile markdown links without repeated suffix searches (#20322)	2026-09-12 01:29:14 -07:00
Neil	3bc631dad3	perf: validate mobile review table delimiters by cell (#20317)	2026-09-12 01:29:11 -07:00
Neil	cd8e98fdf9	fix: prevent mobile markdown parser from stalling on unsupported blocks (#20313)	2026-09-12 01:29:09 -07:00
Brennan BensonandMerge Sim	9a56797486	fix(mobile): surface host create warnings and terminal-create errors (#20125) ... * fix(mobile): surface host create warnings and terminal-create errors A workspace created from the phone could land on "No tabs in this session" with a bare red "Failed to create terminal" and no way to tell why. Two independent drops hid the host's own explanation: - createWorktreeWithNameRetry returned only {worktreeId, name}, discarding worktree.create's `warning`, and hostNewWorktreeSessionRoute built the session route with only `name` + `created=1`. The session screen has always had the banner (MobileSessionContentRow + createWarningState) -- only the tasks create path ever fed it, so the New Workspace path could never report a startup terminal that failed to spawn. - handleCreateTerminal collapsed every failure to the literal 'Failed to create terminal', throwing away response.error.message. Both now propagate, so the daemon's pty-allocation hint ("Your system cannot allocate any more pty devices.") reaches the phone instead of dying in the main process. Behaviour is otherwise unchanged: a blank warning is still omitted from the route, and a host that gives no reason still reads 'Failed to create terminal'. * test(mobile): refresh route parity baselines --------- Co-authored-by: Merge Sim <sim@local>	2026-09-11 11:21:31 -07:00
Jinwoo Hong	58ff95becb	refactor(mobile): name the RPC acceptance policies call sites hand-rolled (#19960)	2026-09-10 19:37:52 -07:00
Jinwoo Hong	4e1681338c	refactor(mobile): extract settings, diagnostics and editor-document screens from their routes (#19675)	2026-09-10 16:10:36 -07:00
Brennan BensonandMerge Sim	4b4acf26a4	fix(mobile): enable patch-free iOS text selection in native chat (#19769) ... * fix(mobile): make every native-chat text node selectable Long-press selection worked on some chat text and not others. Markdown paragraphs — the default block for agent prose — were the one block type left out when headings, quotes, code, lists and table cells gained `selectable`, and tool result output, diff rows, the unloadable-image placeholder, permission/question bodies and the send-error banner never had it at all. Selection is now set on every content Text in the chat surface, on the outermost block Text so nested inline spans inherit it. Labels inside a Pressable (option rows, tool-line headers, buttons) are deliberately left alone: selection there would swallow the tap they exist for. Extracting MobileNativeChatEmptyState keeps the view under its max-lines cap and matches desktop, where NativeChatEmptyState is already its own component. Tests render each surface and assert selection on the block that carries the prose; both files were ablated against the unfixed source (4/10 and 3/5 red) so they pin the defect rather than the current behavior. * fix(mobile): support native text range selection on iOS * fix(mobile): remove persistent assistant message controls * fix(mobile): scope patch-free text selection to chat Use the stock react-native-uitextview dependency behind an iOS adapter and opt assistant Markdown into range selection only in native chat. Preserve the existing React Native Text behavior elsewhere and remove the persistent assistant controls. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-09 21:50:31 -07:00
Jinwoo Hong	d936d8da82	revert(mobile): pull the relay connect-speed mobile pass pending a smaller, verified re-land (#19348) ... * Revert "feat(mobile): time relay dial stages so diagnostics say where a slow connect went (#19245)" This reverts commit 83b1558ecc. * Revert "perf(mobile): race the direct and relay dials from t=0 on every reconnect (#19308)" This reverts commit ceafdcad2f. * Revert "feat(mobile): draw the last known tab strip while a session reconnects (mobile pass) (#19281)" This reverts commit 643571def6. * Revert "perf(mobile): open a session with parallel startup RPCs and a pre-warmed terminal engine (#19260)" This reverts commit c37413271e. * Revert "perf(mobile): cut the relay reconnect critical path and admit dead sockets faster (mobile pass) (#19280)" This reverts commit e628090ad4. * chore: keep the react-doctor suppression for the startup timers The pattern it covers (a variable number of timers cleared through one cleanup) predates #19260 and is unchanged by the revert; dropping the entry only re-exposed a pre-existing finding to the changed-code gate.	2026-09-07 16:44:26 -04:00
Jinwoo Hong	c37413271e	perf(mobile): open a session with parallel startup RPCs and a pre-warmed terminal engine (#19260) ... Startup RPCs now fan out in parallel and the xterm engine pre-warms inside the real terminal frame while they are in flight, so the first pane inherits a warm WebView and an already-measured viewport instead of paying a round trip for it. The pre-warm opens its engine before measuring: web-ready only reports that the bundle loaded, and the WebView answers a measure with null until a terminal exists. It also pre-warms at the user's saved text size, because cell size is what the frame height gets divided by. Host writes such as worktree.activate wait for an evaluated status.get reply. Navigation still fails open when a host cannot answer one, but that fallback no longer reads as a passing compatibility verdict.	2026-09-07 13:16:44 -04:00
Neil	e9d9d42ccc	perf: find mobile Markdown placeholder prefixes in one scan (#18914) ... * perf: find mobile Markdown placeholder prefixes in one scan * style: format mobile Markdown benchmark guard * docs: explain collision-free Markdown prefix length	2026-09-05 20:02:53 -07:00
Brennan BensonandMerge Sim	b0c67eaf88	feat(mobile): port the restructured native-chat turn status and live tool progress (#18761) ... * feat(mobile): port the restructured native-chat turn status and live tool progress Mobile chat had a single static "Agent is working" row and no live tool activity, while the desktop restructure (#17597, #18705) replaced that with a per-turn status row and a running-tool label. This brings mobile to parity and puts the derivation in one place instead of two. Shared (new, pure, RN-safe — desktop uses them as i18n fallbacks, mobile directly, matching the native-chat-empty-state pattern): - `native-chat-turn-status.ts`: duration formatting, label selection, the turn-timing state machine, and the active/settled split. - `native-chat-tool-activity.ts`: command-tool classification, the running-tool label descriptor, and running-call selection. Desktop now consumes both; `NativeChatWorkingStatus`, `NativeChatToolRun` and `use-native-chat-turn-status` keep their existing behavior and strings. Mobile gains the "Thinking" / "Working for 12s" / "Worked for 3m 4s" row with a caret that discloses the turn's tool activity, the pulsing "Running npm test" row with terminal-vs-wrench glyphs, and desktop's rule that a completed turn's tool run hides behind the turn caret. The bridge lane is untouched and keeps its three-dot indicator. Headings, quotes, code, lists and table cells are now selectable. Files at their max-lines cap were split rather than bumped: the tool-run subtree, the prompt card, the session-lane wiring, and the turn-disclosure state each move to their own module. * perf(mobile): stop the turn-status rows from re-rendering the whole transcript A streaming turn re-renders the chat list many times a second. The disclosure wiring handed every row a fresh status object and a fresh toggle closure on each of those renders, so `MobileNativeChatMessage`'s memo never held and every visible row re-rendered per tick — including settled turns that had not changed. Memoize the status selection on the timing map, and keep one stable toggle handler per turn (pruned when a turn leaves the transcript) attached only to the settled rows that can actually disclose anything. Now only the live turn's row changes identity while the agent works. * fix(mobile): keep the turn clock running when the optimistic echo is replaced An accepted send renders as `pending-N` until the transcript echo lands under its real message id. That flips the active turn key mid-turn, and the timing reducer treated the new key as a new turn — so a turn that had reached "Working for 8s" visibly restarted at "Working for 0s". The reducer now carries the start over when the previous key names a turn that has since left the transcript, which is exactly the echo-replacement case. A genuinely new turn (the previous key still in the transcript) and a turn that had already settled both keep their own clock; both are pinned by tests. Desktop does not pass the new key and is unaffected. * fix(mobile): keep the Tools toggle working on settled turns Hiding a settled turn's tool run behind the turn caret (desktop parity) also made the composer's global Tools control a no-op on every completed turn: the run it wanted to expand was not rendered at all. Let that toggle override the hiding, so it still reveals every run at once the way it did before. * fix(mobile): re-key the turn timing instead of only carrying its start The previous fix carried the start forward only while the turn was still working. When the transcript echo landed after the turn had already settled, the new key inherited nothing, the settled timing was pruned with the old key, and the turn's "Worked for N" row disappeared entirely. Move the timing onto the new key instead, which covers both orderings: an in-flight turn keeps counting from its original start (and later settles against it), and an already-settled turn keeps its duration. Both orderings are pinned. * test(mobile): pin the structured turn-status wiring at the view level Emulator QA could not reach the structured lane (mobile's Create Tab -> Codex falls back to a terminal tab when agentSession.createSupport says unsupported), so the view's own lane wiring had no coverage — the one seam between the shared turn-timing reducer and the rendered rows. Assert what the view hands each row: the live user turn gets a status object and the three-dot indicator is gone on the structured lane; the bridge lane keeps the indicator and gets no status; a finished turn settles to a numeric duration with a toggle; and an assistant row never carries a status row of its own. * fix(mobile): isolate structured chat turn state * fix(mobile): let the capability RPC actually store what a phone advertises `runtime.clientCapabilities.update` records the advertised set by assigning `authenticatedSocket.clientCapabilities`, but the socket handed to the dispatcher defined that property with a getter only. In strict mode the assignment throws `TypeError: Cannot set property clientCapabilities ... which has only a getter`, so the RPC answered `runtime_error` and the set was never stored. The consequence is not subtle: `supportsStructuredAgentSessions` requires the capability, so `projectSessionTabAgentStatus` removed every `agent-session` tab from a phone that had advertised it correctly. A paired phone saw ZERO tabs on a worktree whose only tab was a structured Codex chat — structured native chat was unreachable on mobile over this transport, not just missing its new turn UI. Give the socket a setter that writes through to the channel, which already owns the set for the connection's lifetime, so later requests on the same socket see it. Found while trying to capture emulator screenshots of the turn-status port: two full QA runs reported the new UI "missing" because the phone could only ever get a bridge/PTY tab. * fix(mobile): carry the turn key instead of caching a handler in a ref Builds on the scope-isolation fix: that kept (and extended) a ref that is written during render — once to memoize a per-turn handler, once to prune dead turns, once to reset on a scope change. React Doctor's "Ref mutated during render" is what CI's `check:react-doctor:changed` was failing on (x2), and on mobile it is a real hazard rather than a style note: react-freeze discards renders, and a discarded render would leave the cache mutated. Pass the settled turn's key down the row instead and let it call one stable handler with it. That preserves both properties the cache was bought for — per scope isolation, and identity stability so a streaming transcript does not defeat the row's memo — with no ref writes and no pruning to get wrong. The scope-keyed expanded set and the 128-turn cap are untouched; their tests move to the new contract and one now pins handler identity across a re-render. Note for future changes here: `check:code-quality:changed` does NOT cover this. CI additionally runs the standalone react-doctor CLI, which has rules the oxlint plugin config does not enable. * fix: ship native chat status translations * test(native-chat): pin the shared copy against the English catalog The shared constants are desktop's i18n fallback and mobile's actually-rendered string. If one changes without the other, desktop keeps rendering en.json while mobile renders the constant — and nothing fails, because a fallback is only used when the key is missing. That silent divergence is the exact thing the shared module exists to prevent, and it is now reachable precisely because these strings are runtime-required rather than statically extracted. Assert every key in both shared copy objects matches en.json byte for byte, plus the interpolation placeholders the catalog interpolates on. --------- Co-authored-by: Merge Sim <sim@local>	2026-09-04 23:31:39 -07:00
Brennan BensonandMerge Sim	7f8eb90ac3	Align worktree host labels across desktop and mobile (#18237) ... * refactor: align worktree host labels across clients * fix(mobile): expose safe host display labels * fix(mobile): preserve legacy mixed-host labels --------- Co-authored-by: Merge Sim <sim@local>	2026-09-02 15:32:06 -07:00
Jinjing	512ec1a0da	Revert "Move mobile search button to bottom left floating (#17808)" (#17990) ... This reverts commit 823934034f.	2026-09-01 11:33:18 -07:00
Jinjing	823934034f	Move mobile search button to bottom left floating (#17808) ... * Move workspace search toggle to floating button Extract the search bar into a separate component and move the search toggle button from the toolbar to a bottom-left floating action button, positioned above the new workspace FAB. This consolidates phone-only floating actions in one location. * Remove SearchWorkspacesFab component Consolidates search functionality into bottom-left floating action button as part of mobile search button repositioning.	2026-09-01 02:56:37 -07:00
Neil	2daea491b9	Enable React static component and purity lints (#17337) ... * Enable React purity and static component lints * fix lint follow-up clock activation and eligibility expiry * test render status bar provider panel with hooks * fix checks clock activation before paint * fix React type import in git history files	2026-08-29 23:46:35 -07:00
Brennan Benson	8cf7c6926e	fix(mobile): create-worktree sheet dies after picking a source and loses the picked PR (#16917) ... * fix(mobile): keep the create form on screen through drawer swaps and survive reconnects The create-worktree flow could reach a state where the shared modal host was mounted with no sheet in it: a full-screen transparent window that swallows every tap with no way out. Frame analysis of the reported recording and a live simulator repro both land on the same state - the form sheet laid out at the right frame with progress=1, backdrop painting, sheet not painted. - Keep the form sheet mounted through every drawer transition, so the host Modal is never on screen without a sheet, and drop the render-read pin ref. - Re-assert a pinned sheet's enter transform when it takes the window back from a fill picker; nothing re-applied it before. - Key the form session on hostId, not on the RpcClient object: useHostClient swaps that object on every reconnect, which silently remounted the form and threw away the picked source. - Run the pasted-item lookup concurrently with the provider fan-out instead of after it (measured 2631ms -> 1480ms for a typed PR number). * fix(mobile): remount the sheet view on window hand-back so a rebuilt native view repaints On-device confirmation showed the committed hand-back re-assert never reaches the native view: progress already sits at 1 and translateY at 0, so withTiming produces no style delta, and the dead screen stayed reachable (1/25 on the committed build; 1/9 with a sub-pixel value nudge, which lands on the stale native binding when the view was rebuilt with a new tag). Remounting the sheet's Animated.View on an epoch keyed to the hand-back mounts a fresh native view with the style computed from the current shared values - progress is already 1, so it paints in place with no visible animation. 0 dead in 50 attempts on the remount build under the same churn condition that reproduced the dead screen on base. LANE-REPORT.md carries the full confirmation evidence and limits. * chore: drop the stray lane report from the repo root It is a working artifact, not source, and the root directory guard blocks any new top-level entry. * test(mobile): assert the sheet subtree rebuild directly, not through a test-only prop The hand-back test proved the remount by reading an epoch-keyed nativeID that existed only for it — production markup shaped by a test, and an assertion a future refactor could satisfy without rebuilding anything. Count mounts of the sheet's content instead, which is the property the fix actually depends on, and drop the nativeID. Also stop typing test renderers as 'ReactTestRenderer | null'. The static analysis job installs no mobile/node_modules, so that type is unresolvable there and the union trips no-redundant-type-constituents on every added line. The hand-back re-assert is not dead code as the old comment implied: the drawer swap hands back at 166ms, before the 180ms enter animation ends. * fix(mobile): keep the create form when a render is thrown away The session key was built from counters mutated during render. A blurred screen suspends this subtree (react-native-screens freezes via react-freeze), so React runs the component and then discards that render — but the counter bumps survive it. The next committed render then produced a new key and remounted the form, throwing away the picked source for a host switch or a close that never committed. Hold the open epoch in state, which React discards with the render that set it, and put the host in the key directly instead of counting host changes.	2026-08-28 15:45:54 -07:00
Brennan Benson	5dc09db2cd	fix(ui): label agent state glyphs and swap monitoring to a heartbeat (#16981) ... * fix(ui): label agent state glyphs and swap monitoring to a heartbeat The monitoring glyph read as unlabeled: AgentStateDot set only aria-label, which renders no hover tooltip, so hovering it showed the row's own title — the same truncated text already visible. Its row siblings (agent icon, model chip) both had hover titles, leaving this glyph the odd one out. Give every state a native title in the shared primitive, so done/working/ blocked/idle gain the same affordance across the sidebar, tab bar, dashboard, kanban, cmd-J palette and AI Vault at once. Callers can override via a new optional title prop; AiVaultSessionSubagents drops its now-redundant wrapper. Native title rather than the Radix tooltip: AgentStateDot renders in two surfaces with no TooltipProvider above it — the dashboard popout is its own React root, and AgentMapScene — so Radix would throw there. StatusIndicator already sets a native title for the same reason. Also swap lucide Radio for Activity. Radio reads as "broadcasting"; the heartbeat line reads as "still running", which is what the state means. Mobile keeps its documented 1:1 parity with the desktop primitive. Fixes STA-5794 * fix(ui): avoid duplicate agent state tooltips * fix(ui): preserve disabled agent tooltip reason * fix(ui): stop the state dot from shadowing a row's disabled reason The shared AgentStateDot now emits a native title on every state, so at any call site nested inside an element that already has a title, the dot's generic state word wins on hover over the more useful ancestor text. That regressed the sidebar agent row, which carries `sendTargetDisabledReason ?? rowTitle`: hovering the dot showed "Working" instead of the actionable send-target reason. Same guard the review-notes send menu already uses. Also covers three hunks that shipped untested: the Radix opt-outs in ActivityPrototypePage and the AI Vault subagent line's dropped wrapper title both stayed green when reverted, and the suppression test was a `not.toContain` sweep that passed against the pre-fix tree. * fix(ui): preserve heartbeat hover tooltips * fix(ui): preserve lineage drop hit zones * Use styled tooltips for state indicators * Update jump palette tooltip assertions * Limit status tooltips to agents * Restore agent workspace status tooltips * Keep status tooltips on agent indicators * Clarify agent status tooltip ownership * Restore agent-derived workspace status tooltips	2026-08-28 15:34:47 -07:00
Jinwoo Hong	bf5660df51	feat(mobile): add causal network diagnostics (#16837)	2026-08-27 21:42:23 -07:00
Neil	7ee8b5e1a6	Refactor lower max-lines modules (#16760)	2026-08-27 16:10:51 -07:00
Brennan Benson	81ae98e10d	fix(mobile): honor host worktree create retention (#16342) ... * fix(mobile): honor host worktree create retention * fix(mobile): cover malformed worktree retention policy * fix(mobile): fail closed on malformed retention policy * fix(mobile): fail closed on missing dedupe ttl	2026-08-27 16:02:38 -07:00
Brennan Benson	d2a35eebe3	fix(mobile): avoid unsupported Hermes array sorting (#16506)	2026-08-25 17:11:09 -07:00
Jinwoo Hong	0e0a8c943b	fix(mobile): recover Relay connections after resume (#16498) ... * fix(mobile): recover relay sessions on resume * fix(mobile): expedite relay retry on app resume * fix(mobile): keep relay reconnect controller under lint limit * fix(mobile): rebuild relay client after pairing rejection * refactor(mobile): keep relay reconnect policy under lint limit	2026-08-25 16:25:00 -07:00
Neil	aa4c9c707c	Refactor mobile home, worktree modal, and RPC client (#16165) ... * refactor(mobile): split home modal and rpc client * fix(mobile): restore render-phase remount key in NewWorktreeModal The split moved the form-reset epoch from render-phase refs into useState + useLayoutEffect, which changed when the remount key is computed. On the render where visible flips false->true the key was still the old epoch, so the previous session's NewWorktreeModalContent rendered with visible === true carrying stale form state. Child layout effects run before the parent's, so visible-gated hooks (useNewWorkspaceRepositories, useNewWorktreeDrawerNavigation, useNewWorkspaceRuntimeContext) fired for that stale instance before the parent bumped the epoch and remounted. Restore the ref-based computation so the key is correct on the first render where visible flips true, keeping the composite open/client epoch semantics and the file split intact.	2026-08-24 23:50:10 -07:00