749 Commits

Include renames

Author	SHA1	Message	Date
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 Hong	6a0200f416	test(mobile): certify the files page closure, 28 families and 125 goldens (OTA phase C, C3.2) (#21724) ... * test(mobile): certify the files page closure, 28 families and 125 goldens C3.2. The closure is re-derived at this base from the entries esbuild compiles — the two `.web.tsx` files — and matches the design exactly: 28 families, 125 goldens, C1's 22 a strict subset, 6 families and 22 goldens added. Explorer 3441 modules / 304 local / 10 under `src/files`, preview 3666 / 330 / 19, union 342 local. No `mobileWeb.*` family appears, which is the tell that the native switch was not measured. Two tables at the route seam, as C2.6 splits its own: the explorer's one family and the preview's five are separate evidence for two routes with separate grants, and `c3-page-closure.ts` only spreads them over C1's. C1's 22 families are inherited verbatim, not re-derived. Measured here, C2's rule disagrees with 13 of those 103 pins — all 7 in `tasks.smart-source-search`, all 5 in `host-worktree-refresh`, and `worktree-catalog-snapshot`. C5's docstring says 10; corrected in this commit, comment-only. Every family C3 shares with C2 and with C5 is asserted equal object for object, and the three committed pin files disagree on nothing. Class totals: identical 66, result-absent-settlement 47, params-undefined 7, result-absent-stream-release 3, write-ordinal 2. Pinned beside the per-id walk because a table built wrong in a self-consistent way passes the walk. Red-first, both halves: changing one verdict fails the totals and the inheritance check by name; dropping a golden fails drift, totals and the census count together. What 125 certified does not say: `host-worktree-refresh`, inherited from C1, has no byte-identical golden at all, so its 5 hold a class and not bytes. All six families C3 adds have at least one. No scenario replays a save twice, so `files.writeTerminalArtifact` is certified for one round trip and not for idempotency; and no golden here subscribes, because the domain opens no stream. The `config/scripts` precondition now derives both files routes and compares their union against the C1 + C3 tables, with a second case proving each route reaches a strict part of it — without which the union would pass with one route contributing nothing. Also folds pullfrog's open nit: the render check's comment claimed `toContain('readme.md')` proved the encoded round trip, which a truncated path would also satisfy; the url assertion beside it is what proves it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): share the pin-source reader and finish C5's 10-to-13 correction Two pullfrog findings on #21724. `c5-page-closure.test.ts` still said the rule disagrees on 10 of the 103; only `c5-page-closure.ts` moved in the last commit, so C5's two files disagreed with each other and with C2's and C3's. The sentence now states 13 with the same breakdown the other three carry. No assertion depended on the number. While there, the comment records why C5 compares against the imported object rather than the committed text and is still sound: it inlines its families instead of spreading C1's, so there is no spread for an edited entry to launder through. `pinsFromSource` was byte-for-byte identical in C2's and C3's suites — checked, not assumed — and is now one module both import. It sits beside `page-closure.ts` rather than inside it, named for what it reads: the module it would have joined holds pure table arithmetic and this one reaches the file system. The C2 suite's cases and assertions are unchanged; its diff is the deletion of the copy and one import. The shared reader keeps its teeth: making the wrapped-entry capture unmatchable reds the inheritance check in both composed suites, which is the defect the comment describes — three `result-absent-stream-release` pins once went missing that way with an empty mismatch list. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): name which leg of the path round trip each assertion proves Round-1 nits 3 and 4. A second commit rather than a fold into `dc1de6e728`: that one is already committed, and amending is off the table. The render check's comment claimed the url assertion was the proof of the round trip. It is the outbound leg only — what the page encoded into its own history — and a screen that mis-decoded the middle of the path satisfies it and the title assertion both. The comment says that now, and points at where the inbound leg is proved instead: `mobile-file-path-route-encoding.test.ts`, which reads each hazard shape back out of the href, and `mobile-file-preview-route.test.ts`, which drives the normalizer the screen reads its params through. Both files checked to exist, and read, before being cited. The 10-to-13 edit left a 127-character line in a file that wraps at 100; reflowed. It was the only over-length line the C3.2 commits introduced — the others in both files predate this branch, and `oxfmt` accepts them because it does not reflow comments. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what the inheritance case is blind to, and finish the reflow Round-1 item 3, plus a correction to my own claim. The "inherits C1's families whole" case reads C1's committed text, which makes it independent of the spread but not of C1's file: edit a verdict there and both sides of the comparison move together. The docstring now says that, says what it does catch — a C3 half redeclaring an inherited family, which the spread would otherwise take silently from the last table — and names where the C1-file edit is caught instead. Both measured here rather than taken from the review. Flipping `settings-repo-metadata-icons` in C1's file leaves this case green and reds seven others: the class totals and the exclusion counts in C2's suite and this one, both cross-series agreements, and C5's own inheritance case, which compares against an independent literal rather than a spread. Redeclaring `settings.repo-metadata` in the preview half reds this case along with five more. I also said last round that the 127-character line was the only over-length line these commits introduced. That was wrong: I checked the two files in that commit rather than the branch. Six lines over the 100-column wrap came in across four files, including two I had just written in `c3-page-closure.test.ts`. All six are reflowed, and the check is now over every line the branch adds rather than over the files I happened to touch last. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state both redeclaration shapes, or neither count Round 2's low finding. The docstring said redeclaring `settings.repo-metadata` in the preview half reds this case "along with five more". The reviewer is right that the number is shape-dependent, and right about both figures. Re-measured, no truncation this time: one golden under the family's name reds seven, because it also shrinks the census to 114 and leaves that family with no byte-identical golden; the family copied verbatim with a single verdict flipped reds five, the census unmoved at 125. The docstring states both with the reason they differ, and says plainly that the count is not the claim — the load-bearing half is that the spread takes the last table's entry and this case is what sees it, which holds in both shapes. The six came from reading a `head -6` of the failure list as the whole of it. That is the same mistake in miniature as the one this file's own comments warn about: an empty-looking result that was only a truncated one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 17:18:38 -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	76063e7ab1	test(mobile): certify the tasks page closure, 70 families and 266 goldens (OTA phase C, C2.6) (#21712) ... * test(mobile): read a page closure's run totals through one reader The C5 gate counted the run's classes inline. C2 needs the same count over its own closure, and two spellings of "what the run tallied" can disagree while both stay green, so the loop moves next to `pageClosureTotals` where the table-side count already lives. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): certify the tasks page closure, 70 families and 266 goldens C2 moves the tasks screen to the web, so the goldens recorded at a call site inside `app/h/[hostId]/tasks.web.tsx` and `app/h/_layout.tsx` are the ones whose divergence would be this domain's. Each is pinned by id: the suite's own counts run over 787, where one of the other 521 can pay for a closure golden that stopped replaying. C1's 22 families are inherited verbatim rather than re-derived — C2's rule disagrees with them on 10 of the 103 — and the rule decides only the 48 this domain adds. The pin is split at the domain's seam, one work item opened versus choosing which to open, because the table is 409 lines of data and `max-lines` is not a thing to disable. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): correct the C2 pin's inheritance count and census scope Two comments overstated what was measured. The rule disagrees with 13 of C1's 103 inherited pins, not 10 — the 10 was copied from C5's file, which carries the same error over the same 22 families — and the breakdown is now named so the number can be re-derived rather than trusted. The census reads the committed table and does not re-derive the closure, so a golden arriving in a pinned family is caught while a new family entering the closure is not. That was true and unsaid, which is the worse of the two. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test: derive the tasks page closure's family set instead of trusting the pins Round 2 folds, three. The pin tables walk the families they already hold, so a scenario recorded at a call site the route already imports lands in a family nobody pinned and every assertion stays green. `mobileWebAppRouteClosure` runs in a quarter second and `config/scripts` already imports it, so the derivation is now a test: the family set the closure reaches must equal the union of the three committed tables. C2's inheritance check read the object its own table spreads, which cannot disagree with itself; it now reads C1's file as text. What that does and does not hold is written down, because a verdict edited inside `c1-page-closure.ts` is green there either way — C2 inherits whatever C1 commits. The gate's C1 block gains the run-totals assertion C5 and C2 already had, which is the check that edit does fail. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: run the page-closure family check in the job that installs mobile deps Its closure half asks `mobileWebAppDependenciesPresent()` first, so outside the `mobile_web_app` job it skips itself and the precondition it exists to be never runs. That job sets `ORCA_MOBILE_WEB_APP_DEPS_REQUIRED`, which turns the same question into a failure. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 15:17:29 -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
latteandNeil	bb2afe1792	fix(ai-vault): discover and parse Devin sessions on Windows (#21337) ... * fix(ai-vault): discover and parse Devin sessions on Windows, restore workspace mapping Devin sessions never appeared in the AI Vault on Windows, and parsed nearly empty elsewhere: - The transcripts root hardcoded the XDG layout (~/.local/share/devin/cli/transcripts), but Devin CLI writes under %APPDATA%/devin/cli/transcripts on Windows. The root is now platform-aware (APPDATA on win32, XDG_DATA_HOME elsewhere) for both local scans and win32 remote hosts, and APPDATA joins the scanner child's env allowlist so relocated AppData resolves. - The parser read metadata.is_user_input / created_at / metrics, which real ATIF-v1.7 transcripts don't carry. It now also accepts the ATIF step shape (source, timestamp, step-level metrics/model_name, plain-string message) while keeping the legacy shape. - ATIF transcripts carry no working_directory, so sessions couldn't group under a workspace. The sibling sessions.db index is now merged through the existing sidecar seam: it fills cwd/title/model/ timestamps, honors the db's hidden flag, and re-merges on db-only changes without re-reading transcripts. * fix(ai-vault): inline Devin transcripts root, harden parser/db edge cases - Resolve the platform-aware Devin cli dir in agent-sources instead of importing the shared devin-cli-data-dir module, which is not part of this change (broke typecheck). - Exclude source:'system' steps unconditionally, even when legacy metadata fields would classify them as user/assistant messages. - Guard unix-seconds conversion against out-of-range values so a single bad sessions.db row cannot mark the whole index unreadable. * fix(ai-vault): watch sessions.db-wal so live Devin metadata cannot go stale In WAL mode, committed rows sit in sessions.db-wal while sessions.db keeps its stat until checkpoint, so keying the dependency on the db alone could serve a stale index. The dependency now observes the wal when one exists; the reader still opens sessions.db itself. * fix(ai-vault): probe sessions.db-wal through the WSL-gated stat existsSync bypasses wslGatedStat and can hang a scan on a stalled 9P mount; the fs-import guard forbids it in session-scanner modules. The dependency path resolution is now async and probes through the gate. * fix(ai-vault): honor zero metrics and array messages in Devin steps - firstDevinMetricValue skipped explicit numeric zeros, letting a lower-priority positive metric win and overstating token totals. - ATIF allows step.message as an array of content parts; route it through extractContentText so those steps still feed title/preview. * test(ai-vault): cover array-valued ATIF message extraction The extractDevinStepText fallback that routes an array-valued step.message through extractContentText shipped without a fixture that produces that shape, so a future refactor could silently drop the branch. Pin that an array of text parts feeds the step's title and preview. * fix(ai-vault): invalidate old Devin caches and bound database retries * Discover current Devin ATIF exports alongside legacy transcripts * Recognize drawn geometry in the browser markup contract test * Deduplicate Devin exports across transcript roots * Account for the workspace sleep-state reader in scan budget * Align OMP integration tests with recorded-path resume * fix: update scan benchmarks and await relay environment test --------- Co-authored-by: Neil <neil@stably.ai>	2026-09-19 03:32:12 -07:00
Jinwoo Hong	211821dc17	feat(mobile): render agent session history from the desktop's bundle (OTA phase C, C5.1) (#21596) ... * fix(mobile): refuse a page target the shell will not take instead of opening it here `useRouteHandoff`'s web sibling answered two things — handed off, or push it locally — and fell through to the local router for three different reasons. Only one of them is a page route. An href the protocol's own pattern drops and a shell that answered no are the page reaching past what this shell can serve, and the bundle carries every route under `app/h`, so the fallback does not paint Unmatched: it mounts `session/[worktreeId]` on React Native Web inside the shell. The outcome is now tri-state. A target outside `pageRoutes` is never pushed locally; the page stays where it is and names the reason once per client, which is the bound the other page-side reporters take. Proved in the render check against the real bundle: with the double granting no `navigate`, "Back to hosts" left the host route for `/` and painted Unmatched before this, and now stays put, posts nothing and reports no page fault. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): encode the host id the worktree row's navigation actions build Both targets this sheet offers interpolated `hostId` raw — the C1.2 class the C1.8 stack fixed at the route files and `web.tsx`, at the last two sites that still had it. `useLocalSearchParams` answers the decoded value, so a deep-linked id carrying `?`, `#` or whitespace stops being one segment. It matters more from C5.1 on. Inside the page these targets go through `useRouteHandoff`, which matches the pathname against the shell's `pageRoutes` before deciding anything, and the id is the segment the pattern is reading. The worktree id was already encoded at both sites; this makes the host id match, and the new test pins all four targets rather than only the one that moved. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): render agent session history from the desktop's bundle (OTA phase C, C5.1) The second page route. `agent-history/[worktreeId].tsx` already shipped in the bundle with its own chunk, so listing it adds nothing to the download and moves no route count: the switch is `index.tsx`'s, and the shell still decides, because a bundle naming a grant this app lacks renders the native panel instead. Its `.web.tsx` sibling is required for `index.web.tsx`'s reason — the native file reaches OrcaMobileWebShellView, whose requireNativeViewManager runs at import and takes the whole bundle down in a browser, since the manifest imports every route. First route with two dynamic segments, so both are encoded. Grants are `navigate` and `storage`: a resumed session opens the native session screen, the worktree list now reaches this screen without leaving the page, and `app/h/_layout.tsx` reads the app's own sidebar width above every page route. The panel's router becomes `useRouteHandoff`, which is the seam that tells those two apart: agent history is a page route and is pushed here, the session screen is not and goes to the shell. The three writes a resume makes needed no page-side handling and have none. What they needed was a test that the descriptor's handling survives the extra hop, so each is run through the bridge and against the same fake directly and the two verdicts compared: a refused create raises the host's message, and a lost reply or a shell disposed mid-flight stays delivery-unknown rather than becoming a failure a user would retry blindly. No golden covers those three. The flag census grows its first entry since C1.3, which is what it is for. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin the agent-history Back button to the shell handoff C5.1 wired this button by swapping the panel's `useRouter` for `useRouteHandoff`; nothing else was needed, because `RouteHandoff` is the router's own shape and the seam's web sibling decides `back`. So this commit is the test that would have caught the wiring being absent, not a fix. Red before the merge, green after, on the same four cases: on `e897e8123a`, where `back` was still expo-router's own spread member, 3 failed and 1 passed — the one that passed is the local-pop case, which is the branch C2.2 did not change. After the merge brought in C2.2's `back`, all 4 pass. The pre-merge run named the notify by its literal `'navigate-back'` because the contract constant did not exist yet; it is the same string `BRIDGE_NAVIGATE_BACK_NOTIFY` holds, so the two runs asked the same question. Both module substitutions are the builder's own rather than conveniences: the web bundle resolves `route-handoff` and `client-context` to their `.web` siblings, so mocking each to its sibling gives this screen the module graph it has inside the page. The frames are read off the port pair's lane rather than off a spy, and one case asserts a frame crossed at all before either absence is read as an answer. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): compare the resume's second write, not only its first Round 1, finding 1. All three `resumeAiVaultSessionInTerminal` cases settled the create (`ai-vault-resume-launch.ts:158`), so `terminal.send` had never crossed the bridge and the half of the resume that types the command into the pane was uncompared. Three cases now drive both writes: a refused send raises the host's message, an accepted send reporting `accepted: false` in-band says "Terminal input is locked", and a send the host takes resolves — the last one being the presence precondition, since a run that failed at the create would give the same shape of verdict as one that failed at the send. Reading `requests[1]` straight after settling the create finds nothing on the bridged leg: the second write is made only once the first settles, so it is two more lane round trips away. `nthRequest` waits instead, and says how many it saw when it gives up, so this cannot pass by proving the opposite of what it says. The locked reply is `{ send: { accepted: false } }`, not `{ accepted: false }`: the reader is `reply.send?.accepted !== false` (`review-terminal-reply-schema.ts:65`), and the flat shape resolves rather than throwing. Written the flat way first, both legs agreed on "(resolved)", which is the comparison doing its job. Also finding 1's second half: the file docstring claimed every case runs twice and differences the verdicts, which was false for the dispose case — a fake RPC client has no door to shut, so there is no native run to compare against. The docstring now says so and the case carries the same note. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin what encoding cannot save about a dot-segment id Round 1, finding 2. The route's docstring listed `/ ? #` and whitespace and the encoding test covered five ids of that kind, which together implied encoding makes any id safe. It does not: `encodeURIComponent('..')` is `'..'`, so the pathname reaches `BRIDGE_ROUTE_PATHNAME_PATTERN` intact, fails the lookahead that stops a climb out of `/h/` (`bridge-caps.ts:68`, read through `bridge-envelope.ts:117`), and the shell answers with `reportShellFailure` — a failure screen where the route would otherwise have rendered the native panel it already has. Pinned, not fixed, and the docstring now says which. `app/h/[hostId]/index.tsx` builds its pathname identically and has the same hole, so this series fixing one of two call sites would leave the shape behind and stop describing it. The new case asserts both halves — the segment survives encoding unchanged, and the pattern refuses the pathname — so a later change that starts encoding dots fails here and has to say which screen it wants instead. Characterisation, so it was green on the first run rather than red: the claim is about behaviour that already ships, and the value is that the refusal is on record. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): mount the agent-history route in a real browser Round 1, finding 3. Nothing rendered this route's real module graph anywhere. The unit tests mock react-native, safe-area, svg, lucide and the icon assets away — they have to, since react-native is Flow source vitest cannot parse — so a component in this closure with no web build would have reached a device before it reached a test. The render check is the only place the graph meets React Native Web, and this route was not in it. Two cases. The first mounts the route from the shell double and reads the screen: "Agent Session History" and the worktree label the params half carried, no fault, no console error, no CSP refusal, and the URL the page wrote for itself. That also proves `init.route.params` end to end on a route that has a dynamic segment too, which §1 of the design claimed and nothing checked. The second pins the chunk. C5 is the first series whose success path pulls a second chunk after the first paint, which on iOS goes through WKURLSchemeHandler under `script-src 'self'`. The chunk is named from the builder's own route map rather than guessed from the bytes, and asserted absent from what the first route loaded, so this says the route came over the wire now. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say the real lifetime of the route-handoff refusal set Round 1, finding 4. The comment claimed one line per reason "for the life of one client", borrowing `createPageDiagnosticReporter`'s bound. The set is built inside the `useMemo` keyed on `[client, router]`, so it is per hook instance: in practice the memo is not recomputed, because `useRouter()` is expo-router's module singleton and the page holds one client, but every screen calling the hook gets its own set and a reason can be reported once per screen rather than once per document. Says that now, and why it is not tightened: a per-module set would outlive the page's client, which is the lifetime the rest of these reporters are scoped to, and there is no document-wide reporter to join without reaching into a contract file the C2 lane owns. Records the other half of the finding too, which came back confirmed rather than changed: `console.warn` is right here. It is the vocabulary `page-bootstrap.ts:35` already writes in, and a `fault` notify would be wrong twice — the shell drops the generation on a page fault, and a navigation the page declined is not a failure. Comment only; no behaviour change, 25 navigation tests unchanged and green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): wrap every router member that takes a target, not three of five Round 2, finding 1. `...router` hands through everything this file does not name, and two of the members it did not name take an href: `navigate` and `prefetch`. `navigate` to a route outside `pageRoutes` went straight to expo-router and pushed it into this document — the hole the tri-state exists to close, reopened under a name nobody had looked at. No call site uses it today, which is why it shipped. `navigate` is now wrapped exactly as `push` is: which of push-or-collapse it does is a decision about this document's stack, and a target outside this document has no such stack. `prefetch` is decided the other way, explicitly. It is the one target-taker that must never reach the shell: a prefetch is a background load, `navigate` is the only thing the shell can be told, so handing one over would open a screen nobody asked for. A route this document serves is prefetched here, which is what the per-route chunk split makes worth doing; every other one is dropped without a line, because a warm-up that did not happen is not a failure to report. The docstring's "four members that can leave this document" is now five wrapped members and a rule for which is which. A list would rot, so the pin is derived: `HrefTakingRouterMember` reads the parameter tuple of every member of `RouteHandoff` and `WRAPPED_HREF_MEMBERS` is asserted equal to it in both directions. It reads the tuple rather than testing assignability because `() => void` is assignable to `(href: RouterHref) => void`, which would make `back`, `dismissAll` and `reload` target-takers and prove nothing. Checked both ways: dropping `prefetch` from the list fails the compile with "Type 'HrefTakingRouterMember' does not satisfy the constraint", and the union resolves to exactly the five, with `back` and `setParams` outside it. The pin is in the product module because `mobile/tsconfig.json` excludes tests. The runtime test asserts each wrapped member is not the router's own function and that `setParams` still is, so a hook that wrapped everything fails too. Red first: 4 of the new cases fail against the previous file, 33 pass now. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): say per hook instance in the title too, not per client Round 2, finding 2. The source comment was corrected in round 1 and this test's title was not, so the two disagreed about the bound the refusal set actually has: the set lives in the `useMemo`, so it is per hook instance, and a title claiming per client is the stronger promise the code does not make. Title only; the case and its assertions are unchanged. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what the agent-history render case does not cover Round 2, finding 3. The docstring claimed the case is where the panel's closure meets React Native Web, which overstates it. The shell double answers no RPC, so the session scan fails and the panel paints its "Unable to Load" state: the session list, its rows, the resume button and the scope tabs never render, and a render-time gap inside any of them would pass this check. Now says both halves — import-time evaluation of every module in the closure and the panel's own chrome are covered, the list subtree is not — and names what covering the rest would take: a double that answers `aiVault.listSessions`, which is a different instrument and would put domain behaviour in this file. Text only. This case moves to its own file on the extracted harness after the merge with #21592; the corrected text travels with it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): import the handoff module once in its own test My round-2 fold added `WRAPPED_HREF_MEMBERS` as a second import of `./route-handoff.web`, which `import(no-duplicates)` fails in the focused-plugins pass of the changed-code gate. Joined to the existing import below the mocks, which is where an import of the module under test has to sit in this file. Found by running the changed-code gate rather than by review: mobile tsc, whole tree oxlint and the suite were all green with it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): give agent-history its own render file on the extracted harness The render check gained browser cases from three domain series at once, each under the `.mjs` cap of 600 counted lines alone and no two of them together: at 39b15e395d the file was 781 raw lines and clean, main's was 786 and clean, and their merge was 857 raw and 623 counted, which is the CI red on #21596. C1.10 extracted the harness so a domain gets a file instead. This is C5's, and the render check is back to 639 raw lines and clean. Five cases. The two that moved — the route mounts and paints, and its chunk is fetched on navigation — plus three new ones. Back, twice. With `navigate` granted the page's Back control posts exactly one `navigate-back` notify and the page does not move; with the grant withheld the same tap reaches the same handler and posts nothing. The pair is the point: the document holds the single history entry the entry wrote with `replaceState`, so a Back this page served itself would also have gone nowhere and looked identical. This is the first proof of that handoff in a browser rather than against a mocked router. And a row. The harness's new `replies` lets the double answer named methods, so the panel now renders a real session instead of its "Unable to Load" state, which is the render-time gap the round-2 docstring conceded. Assertions are on the row's own text and message count, plus the absence of both silent states — the scan failing, and a session out of scope. Replies lifted from the corpus, and one of them needed two scenarios. The session and worktree lists are `aivault-history-screen-listed`'s. Its `status.get` is a capability list alone, and the first run painted "Update Orca on your computer": `HostProtocolGate` above every host route reads the same method for fields that scenario never scripts. The status reply merges those from `transport-host-status-gates-ready`, and the comment says why two. `wt-history` is load-bearing, not incidental. The panel opens on the `workspace` scope and filters by paths from the worktree list, so on any other worktree these same replies paint "No agent sessions" — green, and proving nothing. Registered in the `mobile_web_app` job beside the drawer check, which is the job that makes a missing mobile install fail rather than skip. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): encode the host id at every href the host page builds Pullfrog on #21596. My earlier commit fixed the two hrefs in the row's navigation sheet and stopped there; five more sites in the same page interpolate the decoded id raw — Accounts and Tasks in both header layouts (`host-screen-header.tsx:188,204,307,318`) and the session target `openWorktreeSession` builds (`use-host-worktree-actions.ts:192`). Same C1.2 class. The persisted host store admits any non-empty id and both `useLocalSearchParams` and the store answer it decoded, so one carrying `/`, `?`, `#` or whitespace stops being the single segment `matchesRoutePattern` reads. Inside the page that decides where a tap goes, because the handoff matches the pathname against the shell's `pageRoutes` before choosing this document or the native stack. A census rather than five more assertions: the failure is a habit, not a bug — each of these was written by copying the one beside it, and the seventh will be too. It counts `/h/${...}` interpolations across the host page's four source files and requires `encodeURIComponent` at each, with a presence check so it cannot pass on an empty list. Two sites are exempt and stay raw: `use-host-worktree-actions.ts:171` and `app/h/_layout.tsx:100` compare against a pathname the router answers rather than building a link, so encoding them would change what a comparison matches instead of what a tap opens. The exemption is subtracted by count rather than matched away, so a file that lost its comparison and gained a raw target does not come out even. ONE BEHAVIOURAL EDGE, named rather than fixed. `navigateFromHostList` short -circuits when `pathname` equals the target minus its query. That comparison now has an encoded target on one side and whatever `usePathname()` answers on the other, so for a host id that needs encoding the short-circuit stops firing and a tap on the screen you are already on re-navigates instead of doing nothing. It is a redundant navigation, not a wrong one, and the guard at :171 is unaffected because it compares against the same raw form it always did. Left alone because fixing it means deciding what `usePathname()` returns for an encoded segment, which is a question worth its own change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): guard the optional host id the session target encodes The commit before this one did not typecheck: `useHostWorktreeActions` takes `hostId` as `string | undefined`, and `encodeURIComponent` does not. I committed on a green test run without waiting for `tsc`, which is my error and the reason this is a second commit rather than an amend — the lane forbids rewriting a commit that exists. `?? ''` rather than a cast or a non-null assertion. An absent id then builds `/h//session/...`, an empty segment the shell's own route rule refuses, instead of the string "undefined", which that rule would accept as a host genuinely named undefined. Every other member of this hook already guards the same field. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): keep the agent-history route native when the bridge would refuse its id CodeRabbit on #21596. The persisted host store admits any non-empty id, so `.` or `..` reaches this route, survives `encodeURIComponent` unchanged, and fails the bridge's own segment rule. The route handed it over anyway: `bridge-host.ts` parses the route against `BridgeInitRouteSchema`, drops it to null when it fails, and the page answers an `init` naming no screen with "Update Orca to open this workspace". A failure screen, in place of the native panel sitting right behind this switch. The route asks the schema first now and stays native when the answer is no, which is where every route starts. Mirrors C3.1's call for the files routes (`69e618e19a`), including its reason for using the schema rather than a copy of its bounds: two spellings of one rule drift, and the half that matters is the half the page reads. The pin moves with it. It characterised the refusal before — asserting the pathname was built and that the pattern rejected it — and now asserts the native render, for a dot host id and for a dot worktree id, which is the other segment and was never covered. `app/h/[hostId]/index.tsx` has the same hole and is not fixed here, as asked: it builds its pathname the same way and hands it over unchecked. When C3.1 is also on main the two guards and `mobile-file-shell-route.ts` belong in one module beside the schema, rather than a third spelling of a one-line call. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): forward navigation options through the wrapped router members CodeRabbit (major) on #21596. expo-router's `push`, `replace`, `navigate` and `dismissTo` are `(href, options?)`, and the wrappers took the href alone. A local push asking for `{ withAnchor: false }` reached the router without it, so inside the page the router did something other than what the caller wrote — silently, because dropping an optional argument is not a type error. Each wrapper forwards both on its local branch now. Nothing in this tree passes options today, which is why it went unnoticed and exactly why it needed pinning: the first caller to pass one would have had it dropped without a word. Options do not cross to the shell, and the docstring says so rather than leaving it to be discovered. The `navigate` notify carries an href and nothing else, so a target handed over is opened by the native stack on that stack's own terms. That is the right shape — the options describe a push inside a document the shell's target is not in — but it is a loss, and a loss worth naming. Four existing assertions moved from `toHaveBeenCalledWith(href)` to `(href, undefined)`. That is what the router now receives when a caller passes none, and expo-router reads an undefined second argument as absent; the comment above them says so, so the next reader does not take it for a bug. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): count the wrapped members the way the returned object does Pullfrog on #21596. The header still described the set as it stood before `860577cc30`: "the four members that can leave this document", "the three that carry a target", "the other three". There are six wrapped members now and five carry a target, so every count in the paragraph was one or two short and a reader checking the object against the prose would have found neither explained. Now says six wrapped, five target-takers named and pinned by `WRAPPED_HREF_MEMBERS`, four decided by the shell's route list, `prefetch` the fifth and decided differently for a reason the member's own comment gives, and `back` the sixth carrying no target at all. Comment only; 36 navigation tests unchanged and green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 05:15:46 -04:00
OrcaWin	54a19c2ba3	fix(pdf): render CJK text with pdf.js resources ... Merged after PR-specific checks passed. CI failures are unrelated baseline findings in ClientHostedBrowserPagePane.markup.test.tsx and pane-title-update-global-scan-budget.test.tsx.	2026-09-19 01:28:09 -07: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	428558b941	fix(mobile): let the shell's page paint a file preview (OTA phase C, C3.0) (#21591) ... * fix(mobile): let the shell's page paint a file preview A file preview has one shape on the wire: the desktop answers a base64 body and `normalizeMobileFilePreviewResult` composes `data:<mime>;base64,<content>` for React Native Web's `Image`. Under `img-src 'self'` the browser refuses to load it, so every image preview in the page paints nothing — reproduced in the render check, which logged the refusal naming `img-src 'self'` before this. `data:` is granted to images and to nothing else, so what it admits is what the page itself composed out of a reply it already holds; `script-src 'self'` and `connect-src 'self'` are untouched, and `blob:` is not added because nothing in the closure needs one. Both platform pins narrow from "the header contains no `data:`" to "`data:` appears on `img-src` and nowhere else", which is the check that still fails if a later directive grows one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what the data: image case actually loads through Round 1 is right on both counts. In react-native-web 0.21.2 the hidden <img> the Image component renders carries `alt`, `style`, `draggable`, `ref` and `src` and no load handlers at all — it is there for the browser's image context menu and for `getBackgroundSize()`. The load signal comes from `ImageLoader.load`, which is `new window.Image()` with `onload`/`onerror` on it, so the `new Image()` in this case is the same mechanism the screen's own load runs through rather than a stand-in for it. And the screen maps `onImageError` to "Unable to load preview" (`MobileFilePreviewScreen.tsx:282`); "Binary preview unavailable" is the normalizer's `binary_file` branch, which a CSP refusal never reaches. Comment only. No assertion moves. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): state the img-src data: bound as the destination, not provenance "admits only what the page itself built" read as a provenance guarantee, and CSP has none to give: `data:` is matched as a scheme, so the directive admits any `data:` image URL and the browser cannot tell one the page composed from one it was handed. Nor is the content the page's own — the mime type and the base64 body both come from the host, and `normalizeImagePreviewResult` only checks the mime type is a non-empty string. The true bound is where the URL goes: it is never fetched as anything but an image, `img-src` is the only directive admitting it, an image fetch executes nothing (an SVG inside an `<img>` runs no script), and `script-src 'self'`, `connect-src 'self'` and `object-src 'none'` are untouched. Both copies reworded identically, since they are kept in step by the render check's own policy comparison. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 02:36:39 -04:00
Jinwoo Hong	edd9e3125b	fix(mobile): give the page its height and its long press; C1.7 device proof (OTA phase C, C1.9) (#21589) ... * fix(mobile): give the Route A document the height its mounted tree measures against (OTA phase C, C1.9) The document this builder emits carries no stylesheet, so `html`, `body` and `#root` have no height, and every box react-native-web lays out below the mount is `flex: 1` against a parent that measures 0. The collapse is silent in every check that existed: the entry stamps `mounted`, the route tree commits, `innerText` holds every row, and the accessibility tree reports each one at the offset it would have had. Nothing is painted below the header, and nothing takes a tap — the list sits inside a scroller the collapse clipped, and a phone reads it to VoiceOver while no row responds. Lane C1.7 found it on both an iPhone 17 Pro simulator and a Pixel 9 Pro emulator, and the same bytes reproduce it in headless Chromium. The fix is the reset Expo's own web template ships for a react-native-web root, emitted inline because the shell's CSP already allows `style-src 'unsafe-inline'` for the sheet react-native-web injects at runtime; a linked asset would paint the collapsed layout until it landed. The render check gains the assertion that would have caught it: the root's box measured against the viewport, and the one control this route paints with no RPC answered — the New Workspace button, positioned against the bottom of the root, which the collapse moved to y = -72 — asked for by `elementFromPoint` at its own centre. Laid out is not reachable, so the check is a hit test and not another read of the DOM. Without the reset it fails `expected +0 to be 844`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): let the page have the long press WebKit was taking (OTA phase C, C1.9) The shell's WKWebView is built with the default text interaction, so WebKit installs its selection assistant over the page. A hold on a worktree row raises the selection loupe over the row's own text and the touch is cancelled before the page's responder sees it, which leaves every long-press action in the page dead on iOS while a tap works. Lane C1.7 measured it: the same injected hold opens the row action sheet on the native list and on the page in the Android WebView, and does nothing in the page on iOS. It is not the document's to fix, which the device disproved one rule at a time: `-webkit-touch-callout: none`, `-webkit-user-select: none`, and both together all left the loupe and left the hold undelivered, and headless Chromium confirms the property computes to `none` on the page's text, so the CSS reaches it and WebKit's own gesture wins anyway. The cost is real and named here rather than discovered later: the page has no text selection on iOS, so selectable `Text` — markdown, diff rows, file preview, chat — cannot be selected there until a page-side copy affordance exists. Everything the shell already forbids is unchanged, and Android is untouched. No unit test: the module's Swift checks compile the seven WebKit-free logic files and never import WebKit, so a `WKWebViewConfiguration` cannot be built in them. The device proof stands in. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make the root reset the template's bytes, not a copy with an addition The comment said the reset is what Expo's web template ships, and `margin:0` was not in it. `@expo/cli@55.0.36/static/template/index.html` carries height, `overflow` and the root's flex box and nothing else, and react-native-web emits `body{margin:0}` in the sheet it injects at runtime, so the addition only covered the frames before that sheet landed. Nothing pinned it either: removing it left all 60 tests green, which is the other way of saying it was never load-bearing. Dropping it makes the string one thing with one source instead of a copy to keep in step with two. The pins on the rest of the reset are unchanged, and so is the frame that mattered: the root still has a definite height before the first paint, which is what the collapse needed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile): format the render check with the root formatter `config/scripts` is formatted by the root oxfmt, not mobile's, and CI checks neither, so a 102-char line I added sat over the root's `printWidth: 100` with nothing to catch it. Reflowed by `./node_modules/.bin/oxfmt --write` from the repo root; no behaviour change. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what the root reset shares with Expo's template, not that it is its bytes "The bytes Expo's web template ships" is false and checkable: the template's own block is pretty-printed with comments and trailing semicolons at 410 bytes, and this string is 112. What is actually true, and what the next reader needs, is that it carries the same declaration set and the same `id="expo-reset"`, minified. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin every rule of the root reset, not two substrings of itself The check read the constant back against itself: `toContain(MOBILE_WEB_APP_ROOT_RESET)` plus two substrings taken off that same constant. A rule dropped from it took the assertion with it, so `body{overflow:hidden}`, `flex:1` and the `expo-reset` id were unpinned — and the render check stays green without the overflow rule, so nothing else held them either. Each rule is now a literal written here, named one at a time so a failure says which one went, and the id is pinned beside them. Verified red-first: removing the overflow rule, the `flex:1`, or the id each fails this test and only this test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 02:28:11 -04:00
Neil	d35e1dcba8	ci: route pane close and retirement changes to the close specs (#21572) ... Every terminal-pane route named what BINDS a pane — the pty transports, the ssh reconnect ledgers, the park watchers. None named what unbinds one. #21005 changed the pane close and retirement lifecycle across three transports, and replaying its fifteen paths through the selector returns [] with --reusable-workflow false: it merged with E2E skipped outright. #21001, the same seam a week earlier, ran E2E only because it happened to also touch an ssh-named file, so the close specs were never selected even then. Unbinding is the half that strands a PTY or leaves a retired leaf mounted as a blank pane, so the three specs that judge it now gate it: the parked-tab close retirement, the split-pane close layout consistency, and the paired client's view of a leaf the host retired. Scope held deliberately narrow. The route is not an SSH source route, so it does not start a Docker relay; close reaches SSH only through the shared provider the non-Docker specs already cover. runtime-rpc-client.ts is left out although #21005 touched it: it carries no close decision and churns about three times as often as these files, so routing on it would run this lane on unrelated runtime work. Replaying twenty merged PRs shows exactly one selection change, #21001.	2026-09-18 23:26:01 -07:00
Jinwoo Hong	1ef947394b	feat(mobile): hand the page's dead Back button to the shell (OTA phase C, C2.2) (#21582) ... * feat(mobile): answer navigate-back on the shell side of the bridge (OTA phase C, C2.2) A page served at `/` holds the one history entry its entry wrote with `replaceState`, so `history.back()` goes nowhere and a page Back button is dead. The only stack with somewhere to go is the native one the shell pushed the page onto. Adds `notify { name: 'navigate-back' }` to the closed client union, gated on the existing `navigate` grant rather than a name of its own: an app that can open a screen can close one, and a new grant name would leave every route declaring it native on every shell already shipped. `MOBILE_WEB_SHELL_GRANTS` is unchanged and `BRIDGE_PROTOCOL_VERSION` is not bumped. `bridgeNotifyRefusal` grows a name-to-grant table, since this is the first notify whose name is not its grant's. The shell screen pops its own stack and answers false when there is nothing left, which the host logs as `navigate-back-refused` — nothing crosses back to the page either way, so silence there is indistinguishable from a Back button that worked. Inert until a consumer exists: no page posts the name yet. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): hand the page's dead Back button to the shell (OTA phase C, C2.2) `useRouteHandoff` wrapped `push`, `replace` and `dismissTo` and left `back` as expo-router's own, which inside the page pops a history of one and does nothing. It now pops locally when the document grew a stack of its own, and otherwise posts `navigate-back` for the shell to pop the native stack. This is what makes the Tasks header's `onPress={() => router.back()}` (`src/tasks/mobile-tasks-screen-chrome.tsx`) work once the C2.1 consumer routes that screen's router through this seam; the barrel still reaches expo-router directly, so nothing calls this yet. A shell that granted no `navigate` falls through to the local router rather than throwing out of a tap handler. A shell that granted `navigate` but is too old to know the verb refuses the frame as `unrecognised-message` and logs it; neither is distinguishable from the page, and the fallback goes nowhere in both — which is exactly where Back already went. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): count back among the members that leave the page document The header said three wrapped members driven by one answer. `back` is a fourth, and it is not driven by that answer: it carries no target, so the document's own stack decides it rather than the shell's route list. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): complete the bridge hook probe in its two local literals `Probe` grew `backPops`, and two cases build the object inline rather than through `mount`. `tsc -p tsconfig.json` excludes test files, so only the tests-typecheck ratchet saw it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(config): measure the stack the page Back button rests on (OTA phase C, C2.2) `useRouteHandoff().back()` asks expo-router's `canGoBack()` whether this document can serve a back itself, and that answer is React Navigation's, so no unit test settles it. The render check now measures it in the browser it actually runs in. Read through `router.back()` on `/h/[hostId]/edit`, a real route of this tree whose chevron is expo-router's own back, because the page exposes no handle to call `canGoBack()` on and a global added for a test would ship forever. Measured: the router has nowhere to go on the document the shell opens, and a location change does not give it one either. So the handoff's `canGoBack()` gate answers false for everything the shell or the browser can do to the page, and its local branch belongs to a push the page makes through the handoff itself. The shell double now records every notify the page posts and takes the grant list as a parameter, so a control that handed something to the shell can be told from one that did nothing. The first case asserts a real tap crossing the bridge, which is what makes the two absences after it evidence. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): refuse a second stack pop while the first is still queued (OTA phase C, C2.2) `canGoBack()` and `back()` disagree about time. The first reads the committed navigation state; the second only adds `GO_BACK` to expo-router's `routingQueue`, which `useImperativeApiEmitter` drains from an effect. Two `navigate-back` frames delivered in one native batch therefore both read the stack the first pop has not left yet, both queue, and a three-deep stack unwinds past the screen the page was opened over. The host forwards every notify it is granted, so nothing upstream coalesced them. `useShellStackPop` owns the pop and latches it. The latch clears on the committed route rather than on a timer, because that commit is the first moment `canGoBack()` answers for the stack the pop actually left; a pop that takes this screen off the stack unmounts it and takes the ref with it. `onNavigateBack` now answers `popped` / `nothing-to-pop` / `pop-pending` so the `navigate-back-refused` diagnostic is true for the frame it names, and the log dedupes per reason rather than burying the second behind the first. Driven against expo-router 55.0.18's own `global-state/routing.js`, evaluated verbatim with only its externals stubbed: a mock of `canGoBack` is what hid this. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): make the notify grant table total over the union (OTA phase C, C2.2) The table was `Record<string, string | undefined>` indexed with a plain string, so a notify name with no row returned undefined, read as ungated, and the host acted on a frame it had never granted. Adding a member to the envelope's notify union raised no error anywhere — the hole was silent. Keyed on `Extract<BridgeClientMessage, { type: 'notify' }>['name']` with `string | null` values, an omitted row is now a TS2741 on the table itself, and `bridgeNotifyRefusal` cannot be asked about a name the table has no row for. Adds the `navigate` and `storage` rows, which were missing: the host was enforcing the navigate grant for `navigate-back` but not for `navigate`. Both are inert while every page is offered every grant, and load-bearing the moment a grant is per-route. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(config): assert no page errors in the browser case that drives popstate (OTA phase C, C2.2) The case that dispatches a synthetic `popstate` read its answer as "the page did not move", and a throw under the page's fault boundary leaves the page exactly there. Without the errors assertion the other two cases carry, that absence was not evidence of what it claimed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the routing module's members instead of asserting them The casting gate refuses the two assertions the loader used, and it is right: a stub that stopped covering an import would have left the members undefined and the test would have driven a half-evaluated module. Destructure and check instead, so that case says so. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): name the two pops the stack latch never hears about (OTA phase C, C2.2) The comment read as if the clear were exhaustive. It is not: a pop landing on an equal pathname does not transition `usePathname`, and a `GO_BACK` the queue discards commits no route at all — `routingQueue.run` shifts every action off the queue whether or not `ref.current` is set. Kept the pathname clear rather than moving to the navigator's `state` event. The event would cover the first stick: `@react-navigation/core` 7.17.2 emits `state` from an effect keyed on the navigator state object, and every pop replaces it. It would not cover the second, which changes no state. And the emitter is the navigator, not the routing module this hook is written and tested against, so the switch cannot be earned by a test here the way the queue behaviour was — it would rest on a mock of the signal under test. Both sticks are bounded instead, in the commit that makes the latch one per stack: the holder releases on unmount, so a stick lasts at most as long as the screen that took it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): hold one pending stack pop per stack, not per screen (OTA phase C, C2.2) `MobileWebShellScreen` mounts at both `app/h/[hostId]/index.tsx` and `app/h/[hostId]/web.tsx`, and `/h/a/web` is deep-linkable over `/h/a`, so two shells can be mounted over one native stack. A latch per screen left each of them holding its own, and two frames from two pages still unwound two screens. The latch is module-scoped and carries which screen took it, so a shell whose own route commits cannot release a pop another shell is still waiting on. The holder also releases on unmount, which is what bounds the two pops the pathname clear never hears about: a latch nobody is left to release would outlive the stack it guards and leave Back dead for the session. Both screen suites now unmount their trees between cases, because a tree that is only dropped is a screen still holding whatever pop it took. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the gated-notify name list nothing reads (OTA phase C, C2.2) `BRIDGE_GRANT_GATED_NOTIFY_NAMES` existed so a caller could ask which names ride a grant. Once the table became total over the notify union, the table itself answers that and the only readers left were the two assertions that read the list for its own sake. Deleted with them. The behaviour they stood next to is kept: the protocol's own names are still asserted ungated through `bridgeNotifyRefusal`. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say what the routing-module guard actually catches (OTA phase C, C2.2) The comment claimed the guard catches a stub that stopped covering an import. It does not: evaluating the module with every stub dropped still defines all three exports, because the module assigns them whatever its imports resolved to, and the failure surfaces later as `TypeError: Cannot read properties of undefined (reading 'navigationRef')`. What the guard does catch is an expo-router upgrade that renames or removes one of the three members this test drives. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): keep the second shell mounted across the holder's removal (OTA phase C, C2.2) The harness swapped the tree's root element between a single shell, a fragment of two, and a single shell again. Each swap remounts everything under it, so the callbacks the cases held belonged to unmounted hooks. One of those could still take the module-scoped latch, and the instance that took it was already gone, so nothing was left to release it — the last case in the file leaked the latch into whatever ran next. The root is now one component with a slot per shell, so removing the holder leaves the second shell's instance alone, and every pop is taken through a callback re-read at call time. `afterEach` now asserts the latch is clear by mounting a screen after every other one is gone and requiring it to pop. Without it this leak was invisible: it surfaces only in a following case, and the case that caused it was last. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-19 02:09:09 -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	ac4dc6599b	feat(mobile): the desktop lists a page route, the shell honours it or stays native (OTA phase C, C1.3) (#21502) ... * 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 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 * 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): 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 * 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 * style(mobile): format the web shell route entry 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 21:53:58 -04:00
Jinwoo Hong	b8f3b1ec00	feat(mobile): the shell names the screen, and the page routes to it (OTA phase C, C1.2) (#21501) ... * 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 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 * 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): 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 * style(mobile): format the web shell route entry 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 21:21:48 -04:00
Jinwoo Hong	d043cfbbee	feat(mobile): the page mounts on the shell's init, with the client injected and a fault boundary (OTA phase C, C1.1) (#21500) ... * 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 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 * 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): 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 * 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 20:45:03 -04:00
7 people	09073086a8	feat(terminal): inline images via @xterm/addon-image (perf-first) (#19512) ... * feat(terminal): inline images via @xterm/addon-image, perf-first Add opt-in inline terminal images (SIXEL, iTerm2 IIP, Kitty graphics) through @xterm/addon-image, designed to keep idle terminals unaffected. Performance: - The addon (base64-inlined wasm decoders + protocol handlers) loads off the boot critical path via a deferred loader that mirrors the WebGL addon: primed after first paint only when the setting is on, read back synchronously at attach, with a 3-attempt cap so a transient failure never disables images for the session and a missing chunk never refetches per pane. renderer-boot-graph guards against eager import. - enableSizeReports:false so the addon never sets windowOptions and double-answers Orca's own CSI 14t/16t responder. - Perf-tuned decode/storage limits (storageLimit, sixel/iip/kitty size caps) in one place. Correctness: - Orca's DA1 handler wins over the addon's (last-registered-first), and the default DA1 response never advertised Sixel (;4), so DA1-detecting tools (chafa, img2sixel, viu, timg) never emitted it. The winning handler now appends ;4 while the setting is on, resolved per query so a live toggle changes the next DA1; idempotent against the ConPTY response that already lists it. - ORCA_IMAGE_PROTOCOL=kitty is exported to spawned shells (local, daemon, relay/SSH) and forwarded across the WSL boundary, so image-capable agents can pick an encoder. Unknown image sequences are swallowed by xterm when the addon is detached, so this never garbles output. - Settings toggle (default on) gates rendering and DA1 advertisement. Cross-checked against community PRs #7775, #11706, and #19201 at the end; credited below. Co-authored-by: s546126 <s546126@users.noreply.github.com> Co-authored-by: XRX193 <XRX193@users.noreply.github.com> Co-authored-by: lmsh7 <lmsh7@users.noreply.github.com> * fix(terminal): bound inline image memory and classify Kitty replies * fix(terminal): bound image decode and release image resources on cleanup * fix(terminal): address image addon review feedback * test(terminal): stub setPaneInlineImagesEnabled in appearance manager fakes * fix(terminal): evict unplaced kitty payloads before displayed images Byte-budget eviction dropped the oldest transmitted blob regardless of placement, so a new upload could erase a visible image while abandoned blobs still held budget. Unplaced payloads now go first and displayed ones only when that is not enough. The incoming image is always stored, so an oversized one overshoots the cap by one payload instead of being dropped after the protocol already acked OK. * fix(terminal): gate DA1 Sixel on real addon attachment; claim SSH image spec in CI - DA1 advertised Sixel from the setting alone, so a pane whose lazy addon chunk was still loading (or had failed all three attempts) told feature-detecting tools to emit DCS that nothing could render. Track the attached decoder per terminal and require it before setting the ;4 bit. - tests/e2e/terminal-inline-images-ssh.spec.ts was Docker-gated but claimed by no lane runner, so pr-e2e-gate-contract failed and the spec would have self-skipped green forever. - Reject non-positive PNG IHDR dimensions before decode: they are parsed with signed shifts, so a dimension >= 0x80000000 came back negative and slipped past the pixel-limit comparison. - One resolveTerminalInlineImagesEnabled() for the default-on setting; the four call sites mixed '?? true' with '!== false', which disagree on null. - One readInlineImageResources() walk of the addon internals instead of two copies that could drift against the patched dependency. - Isolate the deferred-attach drain per pane; make the zoom-invariance and backing-storage e2e assertions fail when the feature is dead. * refactor(terminal): one lazy xterm addon loader for webgl and image terminal-image-addon-loader was a structural clone of the webgl one — same memo, attempt cap, and .then(ok,err)-clears-memo recovery. Both now wrap createLazyXtermAddonLoader; each keeps its literal import() specifier so the bundler still splits the chunk (verified against a fresh build: addon-image stays out of the boot graph). * refactor(terminal): name openTerminal's addon flags; pin image addon limits Two adjacent optional booleans could be swapped without a type error once inline images added the second one. * docs(terminal): state the real per-pane image ceiling; drop test ordering dependency storageLimit:32 reads like the pane's budget but keys three pools — decoded pixels, retained encoded Kitty blobs, and pending WASM decoders — so the worst case is ~98 MB per pane with no cross-pane governor. Say so at the constant. pane-inline-images.test.ts's deferred case needed to run first; it now takes a fresh module instead, and the rest prime in beforeAll. Verified by running the file with that test moved last. * fix(terminal): satisfy rebased static analysis gate * fix(terminal): complete casting gate cleanup * fix(terminal): recover failed image addon loads * fix(terminal): bound image decoder allocations --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local> Co-authored-by: s546126 <s546126@users.noreply.github.com> Co-authored-by: XRX193 <XRX193@users.noreply.github.com> Co-authored-by: lmsh7 <lmsh7@users.noreply.github.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Neil <neil@stably.ai>	2026-09-18 16:32:49 -07:00
Jinwoo Hong	209d2d8df6	build(mobile): split the Route A page into per-route chunks (OTA phase C, C1.5) (#21475) ... * build(mobile): split the Route A page into per-route chunks (OTA phase C, C1.5) The page bundled as one 8.16 MB script because every route was a static import. The route manifest now defers each screen behind `import()`, the build is esm with splitting on, and the document loads the entry as a module. What the browser parses before the first route can paint drops from 8.16 MB to 908 KiB; the whole page still weighs the same. Two budgets hold it: the chunk count, which catches a split running away, and the bytes the entry reaches by static import, which catches it collapsing back. The second is the one that matters, and it is measured from esbuild's metafile because only that says which import is static. The RequireContext stays synchronous, since expo-router reads keys() to build the route tree before anything renders. A lazy module cannot answer `unstable_settings` or `ErrorBoundary`, which expo-router reads off the namespace, so a test holds that no route in the subtree exports either. The render check now waits for the route's own text: the entry's mount signal lands while the route chunk is still being fetched. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): read a route's synchronous exports from esbuild, not a regex `export { x as ErrorBoundary }`, `export class ErrorBoundary` and a re-export all reach the namespace without matching the declaration pattern the guard was matching, so the lazy manifest dropped the boundary and the page painted blank. A star re-export is now reported rather than read as clean. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * docs(mobile): say that the entry budget is not a per-route opt-out Measured: statically importing one route already breaks the 3 MiB bound for 5 of the 14. The hatch only works for a layout node, which is the only place expo-router reads a synchronous export from. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * build(mobile): derive the chunk ceiling from the route count 64 was three routes of headroom over the 53 chunks 14 routes measure, so C2's routes would have failed on a number measured before they existed. Four per route plus 16 tracks the measured slope; the entry-bytes bound stays the real budget. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): drop the web entry's dead suspense boundary expo-router wraps every screen in its own, so this one never fires; all nine render checks stay green without it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): pin that a client-side navigation fetches the next route's chunk Goes red with splitting off: the tasks screen paints out of the entry and no new script is fetched. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): name every bundle output by its bytes, not by esbuild's path hash esbuild's [hash] is over the metafile's input keys, which are paths relative to absWorkingDir, so a checkout at another depth or with node_modules as a symlink named a byte-identical chunk differently and shipped a different buildId for one commit. Outputs are now renamed leaves-first to the sha256 of their final bytes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): fail the build on a route the lazy manifest would strip The guard ran only in a test while the docstring said it failed the build. It now runs in bundleMobileWebApp and names the route and the export. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * build(mobile): derive the asset ceiling from the chunk ceiling and the images A flat 128 stopped agreeing with the chunk ceiling at 18 routes, where the asset count would have failed first and named the count instead of the split. Chunks plus images plus the document keeps the chunk ceiling the one that trips. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): split the route-manifest tests out of the bundle builder's The builder's test file passed 600 lines. The route manifest, the synthesized RequireContext and the web entry are their own subject and move together. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): give the export guard the builder's route-source loaders Without .js as jsx the guard reported a React Native .js route carrying JSX as "JSX syntax extension is not enabled" instead of reading its exports. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert the navigation fetches the tasks route's own chunk "some new script arrived" passed on any fetch. The builder now names the chunk each route lands in, read off the metafile, and the check asserts that exact path arrived and was not already loaded. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): resolve a route's realpath before matching it to its chunk esbuild writes metafile input keys after resolving symlinks, so every scratch route tree under /var on macOS reached no output and failed the build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): fail the build when the asset ceiling outgrows the shell's map The derived ceiling had no upper bound, and the native shells return null for a manifest over their own 256 rather than truncating it. At 42 images the formula crosses that at 50 routes, inside what Phase C adds, so the build would stay green while the phone got nothing. The number is read from the contract through esbuild, not restated here. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): cover the two hard stops in the content-addressed naming Both throws only ran through a whole bundle before, where neither can be provoked. A cycle and a route no output claims are now asserted directly; each test goes red when its throw is removed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): exit the app-bundle build on one line, not a stack The route-export guard fails this script by design, and a raw stack put the route and the export name under twelve frames of node internals. Mirrors the verifier's exit; the message is printed as thrown because every throw on this path already names its source. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 11:58:42 -04:00
Jinwoo Hong	381a3da46f	feat(build): Route A, the phone's host routes bundled for the web, dark (OTA phase C, C0.7) (#21449) ... * refactor(mobile-web): share the bundle manifest assembly with a second builder Manifest assembly and the on-disk write move to writeMobileWebBundleTree, and the helpers the Phase C app builder needs become exports. No behaviour change to the shipped bootstrap bundle. The CRLF guard grows two exemptions it needs once it is pointed at mobile/src: the image and font extensions .gitattributes already pins -text, and the gitignored webview engine modules the postinstall writes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(mobile): web entry for the host route tree, and its two transport siblings The entry mounts app/h on react-native-web through expo-router's own ExpoRoot. It lives inside mobile/ so one React resolves, and supplies RpcClientProvider itself: the route tree starts below the native root layout that owns it. route-manifest.ts is a real typed module whose body the builder replaces -- esbuild has no require.context. A virtual specifier would need an ambient declaration and would leave the entry unchecked. Two .web.* siblings, both listed with a reason in web-overrides.json: the transport substitution point (a placeholder client until C0.4 lands BridgeRpcClient) and the device token store, whose native path imports expo-secure-store, which is {} on web. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): build:mobile-web:app, the phone's host routes bundled for the web Same builder shape as the Phase A bootstrap into a separate out/mobile-web-app, with the same manifest and the same two-scratch-build determinism check. Dark: build:mobile-web, packaging and the A2 census are untouched, and C1 is what flips build:release. Six shims, each a named Metro or RN Web gap. Images are emitted as same-origin hashed assets rather than data: URLs, because the shell's CSP sets img-src 'self'; the render check under that exact header is what found it. The script is referenced root-absolute for the same reason a <base> tag cannot be used: the document is served at every route depth and base-uri is 'none'. The budget sits below the contract's per-asset ceiling so growth trips a build rather than a refused asset on a phone. esbuild splitting does not lower it: one entry with only static imports emits one chunk (measured). Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): let React Native Web paint under the shell CSP RN Web 0.21.2 injects its stylesheet at runtime with no nonce support, so style-src 'self' blocks every rule and the page renders unstyled. Measured, not predicted: the render check serves the document under this exact header and reported the violation. 'unsafe-inline' is granted to style-src and nothing else. script-src 'self' holds, which is the directive that decides whether page code can arrive any way other than as a fetched same-origin script. The test now pins that scoping rather than rejecting the token everywhere. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: prove the Route A app bundle on every PR A dedicated job, for the same reason the browser provider has one: it needs mobile/node_modules and a real browser, and the sharded test matrix would pay for both on every shard. It builds the bundle, verifies it, and runs the builder, override-census and render suites. It ships nothing. The mobile_web_app signal is lifted out of should_run the way static_analysis is. A mobile-only diff is desktop-irrelevant and skips every gated job, and that is exactly the diff that changes the page this job builds. Also the C0.6 review follow-up: mobile/package.json and mobile/pnpm-lock.yaml join the installer cache keys in the two workflows that build an installer off a hashFiles key, since beforePack requires out/mobile-web and a mobile-only change must miss those caches rather than reuse a stale build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): pin the shipped builder against the app builder's own module name The assertion named a specifier that no longer exists, so it held vacuously. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): assert the RN Web style-src grant in the Swift checks The Swift twin of the Kotlin CSP test still required style-src 'self' and no unsafe-inline anywhere, so it trapped on the approved grant. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): make the Route A render check name what each route paints The check asserted only "some html, no errors", which expo-router's Unmatched screen satisfies: pointing HOST_ROUTE at /zzz/not-a-real-prefix stayed green. Each route now asserts content only its own component produces, and the unmatched case asserts the screen positively so the negatives discriminate. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): read the shell CSP past the comments that quote directives Both constants document themselves with // comments containing quoted directive text, which the quoted-string scan picked up as directives. One parser now drops comment lines, and iOS and Android go through it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): honour a .web.* route sibling in the app bundle Routes were imported by absolute path with the extension, so esbuild's resolveExtensions never applied and a .web.tsx under app/ was dead code the census still accepted. The manifest now carries a key and a module: the key stays the native filename so the URL does not move, and the module is the web sibling when one exists. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): tie each named shim to the esbuild option that implements it The shim list was asserted against a literal copy of itself, which passes however the build is configured. Each entry now carries an appliesTo that reads its own option, checked against the real options object. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(build): line up the CRLF exemptions, the budget comment, and the job scope The builder loads .gif as a file but neither .gitattributes nor the CRLF scan exempted it, so the blanket eol=lf pin would have rewritten one. A test now keeps the two lists in step. The Phase C byte budget's comment sat on the asset count, and a root package.json edit could change build:mobile-web:app without running the job that proves it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): satisfy the index-check lint rule in the CSP parser Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: key the installer caches on the mobile page trees too beforePack builds the mobile web bundle into the installer. Today those bytes are Phase A's, which src/** already covers, but once C1 flips the entry to mobile/app a page-only change would hit a cache holding a stale installer. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): skip the bundling tests where mobile dependencies are absent The sharded `test` job collects config/scripts/**/*.test.mjs and installs no mobile dependencies, so the two new suites failed there on "Could not resolve react-native-web". They now skip themselves with a message naming the job that runs them, and that job sets ORCA_MOBILE_WEB_APP_DEPS_REQUIRED so a missing install fails it instead of skipping everything it exists to prove. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(build): scan mobile/packages in the .web.* census The census claimed the app entry never resolves into packages/, but the dictation hook imports @orca/expo-two-way-audio and the built script carries ExpoTwoWayAudioModule.web.ts. That file is now listed with its reason, and planting a .web.* in each scanned tree proves the scan is not passing because a tree happens to be empty. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): assert the route exclusions against a tree that has them mobile/app holds no test, spec or +api file, so the exclusion rule was asserted against a tree it could not fire on. A scratch tree plants one of each; dropping the rule now fails this test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): 404 unknown file paths in the render check's page server The server answered every path with the document, so pointing publicPath at /wrong-prefix still rendered three green routes: the script is fetched from the one prefix that is served. A path naming a file now has to come out of the bundle, which is what the shell's manifest map does. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): cover the app bundle verifier's own checks The verifier had no test. One doctors the buildId, which the packaged assert catches; the other rewrites the tree so every digest still agrees and only the two fresh builds can tell, which is what a stale out/ looks like. Deleting either check now fails a test. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(build): tidy the app bundle comments and the job's path prefixes Drops an export nothing read, merges two comments that had drifted apart from the constant they describe, and corrects the claim that the job runs on every PR when it is path-gated. package.json leaves the prefix list because GLOBAL_FORCE_FILES already forces every job on it; mobile/packages/ joins it, since the page resolves a .web.ts out of there. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): merge the duplicate node:fs/promises import in the census Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): redirect the hybrid shell route on the web page app/h/[hostId]/web.tsx reaches OrcaMobileWebShellView, whose module calls requireNativeViewManager at import. In a browser that throws before React mounts, and the route manifest imports every route statically, so one native route left the whole page blank at every URL. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): fail the render check with the error that stopped the mount The check waited on "#root has children" with Playwright's animation-frame polling, so a route module that threw at import read as a bare 30s timeout naming nothing. It now waits on a mount attribute the entry sets after the router commits, polls on a timer, and races the wait against the first uncaught error so the failure carries it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): answer the favicon the render browser asks for CI resolves the runner's Google Chrome, which requests /favicon.ico; the bundled headless shell does not. The bundle carries no icon, so the server answers 204 rather than turning a browser habit into a console error the render assertions read as a page fault. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): settle the render check's uncaught-error race without rejecting The entry throws during goto, before anything awaits the race, so a rejected promise surfaced as an unhandled rejection beside the real failure. The same signal now resolves with the error. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): list the page transport in the raw request port inventory The placeholder client implements the port, so the boundary test counts it as an unlisted file. It belongs under OWNERS until C0.4's BridgeRpcClient replaces it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 09:50:37 -04:00
Jinwoo Hong	ca2ae89011	ci: install mobile dependencies in every desktop packaging job, pin mobile page source to LF (OTA phase C, C0.6) (#21425) ... * chore(mobile): pin mobile page source to LF so a Windows checkout keeps buildId The Phase C web bundle hashes every text byte under mobile/src and mobile/app into its asset digests and from there into buildId. There is no global text=auto, so a CRLF checkout on Windows would give the Windows release a different buildId for identical source, the same failure the src/mobile-web pin above exists for. All 1924 tracked files in those directories are already LF in the index, so the pin renormalises nothing. mobile/web-entry does not exist yet; the pin is forward-looking for the Phase C entry point. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: install mobile dependencies in every desktop packaging job Ten workflows reach build:release/build:desktop and none of them installs mobile/node_modules. Root has no react-native, react-native-web or expo, so once the mobile web bundle builds from mobile/ its packaging jobs would fail at electron-builder's beforePack with an unresolvable import. Extract the frozen mobile install that pr.yml's static analysis job already ran inline into .github/actions/install-mobile-dependencies, and invoke it from every job the packaging census enumerates, after the root install and before the build. Same --frozen-lockfile, same lockfile-drift guard, and still no --ignore-scripts: mobile's postinstall generates the gitignored webview engine modules that tracked source imports. pr.yml now uses the action too, so there is one definition. Where a packaging job's setup-node caches the pnpm store, mobile/pnpm-lock.yaml joins cache-dependency-path so a mobile lockfile change invalidates it. Two jobs (daemon-relocation-spike, win-update-survival-e2e) do not cache at all and are left alone. The census test grows a per-job assertion that the action is present, so a new packaging job has to add the install deliberately rather than discover it at beforePack. release-cut's composite-action restore is no longer Windows-only: every platform consumes this action now, so any of them can be the leg whose cut ref predates it. No job builds anything different; this only makes mobile/node_modules present. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(ci): assert the mobile install contract on the shared action pr.yml's static analysis job no longer carries the install inline, so the scope test's findIndex by step name resolved to -1. Match the step by the action it uses, and read working-directory and --frozen-lockfile off the action itself so the job cannot keep the step while the action stops installing anything. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * chore(mobile): exempt binary asset types from the mobile LF pin /mobile/{src,app,web-entry}/** text eol=lf would mark a future PNG or font as text and rewrite its bytes on a Windows checkout. Exempt the asset types an RN page carries, the same way src/mobile-web exempts its PNG. -text after text eol=lf wins: probed a CRLF-bearing .png under the pin, it stays i/crlf attr/-text while a sibling .ts still normalises to i/lf. No tracked file changes classification; the 1924 files under mobile/src and mobile/app stay i/lf. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * ci: gate the mobile install with the build it feeds in the cached lanes win-crash-survival, win-update-survival and daemon-relocation-spike all skip electron-builder on an installer/unpacked cache hit, so an unconditional mobile install spent time on node_modules nothing then consumed. Move each `uses:` below its cache step and carry the same cache-hit condition as the build. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 05:44:11 -04:00
Jinwoo Hong	f819ed96ca	fix(skills): keep the disposal verdict when staging cleanup fails, and retry release-cut installs (#21366) ... * fix(skills): keep the disposal verdict when staging cleanup fails `begin()` ended with `await this.removeOwnershipIfDisposed()` inside its `finally`, so when a caller raced `dispose()` the rejection it received was whatever that opportunistic `rmdir` threw -- not `skill-upload-service-disposed`. A caller could not tell "the service shut down" from "the filesystem broke", and the Windows release gate saw it as `EPERM: operation not permitted, rmdir`. Two causes, both fixed here: - The EPERM itself: an in-flight operation and disposal each call `ownership.remove()`, so two `rm -rf` run concurrently against the same owner directory. On POSIX the loser reads ENOENT and `force: true` swallows it; on Windows the loser reads a delete-pending directory and gets EPERM. `SkillUploadStagingOwnership.remove()` now joins one removal and forgets it on failure so a later caller still retries. - The masking: cleanup in a `finally` no longer replaces the outcome of the call it is cleaning up after. Disposal retries staging removal and reports its own failure, matching `removeUnpublished`/`retainFailedCleanup` in this class. Both regressions are pinned platform-independently: one injects a failing ownership removal and asserts the racing `begin` still rejects with `skill-upload-service-disposed` while `dispose()` reports the cleanup failure; the other models Windows delete-pending rmdir in the `node:fs/promises` mock, which turns a second removal into EPERM on every platform. * ci(release-cut): retry the installs that fetch node-gyp headers `golden e2e windows` installs with lifecycle scripts enabled, so pnpm runs node-gyp for the `native/windows-registry` workspace project, which downloads that Node version's headers from nodejs.org. A single `read ECONNRESET` on that fetch failed a blocking release gate, and the release build job one screen below already wraps its install in `nick-fields/retry@v4` for exactly this class of failure. Both remaining unretried installs in this workflow (the blocking platform golden and the non-blocking rendering-evidence lane) now use the same wrapper, and a contract test keeps every release-cut install retryable.	2026-09-18 03:12:46 -04:00
Neil	8d2f16856f	fix(session): scope agent resume to the host that captured the session (#21288) ... * fix(session): scope agent resume to the host that captured the session A provider session id names a transcript in one machine's agent state directory. Nothing in the resume path compared that machine against the one the resume executes on, so a record captured on host A reached a `--resume` run on host B, which answers `No conversation found with session ID`. Three things make the drift reachable: `worktreeId` is `repoId::path` with no host component, sleeping records are `'sleepingAgentKeyed'` so boot-time host-contention parking never arbitrates them and every partition merges into one map without retaining provenance, and both issuers resolve their launch target from the current catalog. Both issuers are gated. The activation sweep hands `quit`/`live` records whose pane still exists to the pane's own cold restore, so gating the sweep alone changed nothing in the SSH lane. Declines rather than guesses: the record is preserved and remains resumable by hand. A refused resume is recoverable, a forked transcript is not. The predicate fails open on anything it cannot positively rule out -- an unstamped record, an empty stamp, or a `runtime:` host, which a paired client uses to relabel its host's own SSH workspaces. The cold-restore gate consults both the pane's transport and the catalog. The transport alone was racy: it is unresolved on an early reattach frame, and that frame is exactly when a wrong resume escaped. * docs(session): name the inverted fail-open direction at the resume gate * fix(session): keep an unresolved catalog out of the resume host verdict The worktree form of the resume gate resolved the current host through getExecutionHostIdForWorktree, which answers 'local' for a worktree the catalog has no row for. Read as a host, that made every SSH-stamped record look foreign until its repo row landed, contradicting the module's own contract that it reports only a positively-known disagreement. Add getKnownExecutionHostIdForWorktree, which returns null in that silence (no repo row for a git worktree, no folder-workspace row for a folder workspace), and route the gate through it; the pair form already fails open on a null host. The routing resolver keeps its default unchanged. The CI red on the control case was a separate spec race: the ledger wait returned as soon as the ledger was non-empty, and it already held the first launch's `--version` probe, so the control read two probes and gave up before the cold-restore had typed `--resume` (the failure screenshot shows the command running in the pane). The spec now reads only the lines the relaunch appended, anchors on the relaunch's PTY binding and its own probe, and then waits for `--resume` for the control case or a bounded grace for the refusal case.	2026-09-17 22:12:55 -07:00
Jinwoo Hong	9641a1b544	feat(mobile-web-bundle): serve the packaged mobile web bundle over RPC (OTA phase A, 3/5) (#21348) ... * feat(mobile-web-bundle): serve the bundle manifest and chunks over RPC Two paired-runtime methods on the already-authenticated connection: `mobileWeb.bundle.manifest` returns this install's manifest plus the chunk size it advertises, and `mobileWeb.bundle.chunk` returns one aligned range of one asset with the whole asset's length and hash, so a single chunk describes what it belongs to. `path` is accepted only by exact match against a manifest member, so traversal is unreachable rather than mitigated. Each asset's on-disk sha256 is verified once and the verdict remembered, concurrent first readers sharing one hash. Reads are capped at four in flight per connection, and a disconnected client stops costing reads at the next checkpoint. No SSH or relay proxying: a runtime answers only out of its own install. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): pin the three buildId serializers against each other The canonical serialization exists in the builder, the packaging guard, and the shared contract, because the two packaging scripts run on bare node before any build output exists and cannot import TypeScript. A divergence in any one would reject every honest bundle at packaging, or ship a bundle whose id the phone recomputes differently and re-downloads forever. Proved red by swapping the guard's code-unit sort for localeCompare: five of six cases fail. Exports the guard's serializer for the test; no packaging behaviour changes. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): cover every error code and a multi-chunk paging round trip Against a synthetic bundle in a temp dir, because the real builder's largest asset is under one chunk and CI unit jobs never build out/mobile-web. The fixture's script spans three chunks, its stylesheet is exactly one, and one asset is empty, so paging, the eof boundary, and the zero-byte case are exercised rather than assumed. Reads in flight are held by latching `open`, so the four-per-connection cap and an abort arriving mid-read are deterministic rather than a race with a stopwatch. Both were proved red: dropping the abort check after verification fails the abort case, and keying the cap on connectionId alone fails the device-token case. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web-bundle): resolve the bundle root through the AppEnvironment port check:runtime-electron-ratchet caught this: the resolver sat beside getBundledWebClientRoot in src/main/startup and imported electron, and importing it from an RPC method pulled the first electron edge into a runtime graph whose baseline is zero. The runtime has to stay bootable on plain Node. So it reads app.getAppPath() through the port every other runtime module already uses, and moves next to its two callers under src/main/runtime. A host with no environment installed has no install root, which is the same answer as having no bundle. orcad answers getAppPath from its own install root, so a headless runtime that carries the artifact serves it with no special case. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): cover the resolver's two probe layouts directly Also stops exporting the manifest filename, which nothing outside the resolver needs. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): pin both methods on the mobile allowlist The scanner only checks mobile-used ⊆ allowlist, and no mobile source calls these until A5, so deleting both entries left every test green. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web-bundle): keep filesystem failures inside the six error codes An asset unlinked or truncated after its verdict was cached reached the client as runtime_error carrying the desktop's absolute install path. Both now answer mobile_web_bundle_asset_changed, with the cause warned host-side only. A short positional read is the truncation case, so it throws instead of paging the client past the end. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web-bundle): drop the unreachable release-idempotence guard The one caller releases exactly once in a finally; removing the flag left every test green, so it was defensiveness against a caller that does not exist. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): prove a failed verify is not cached as a verdict The verdict cache never invalidates, so a transient read failure remembered as a verdict would poison the asset for the life of the process. Removing the delete left every test green until now. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web-bundle): delete the unsatisfiable manifest params schema The dispatcher substitutes `{}` for absent params, so `z.null()` could never parse; the method declares `params: null` instead. A comment on the method name records why there is no schema. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web-bundle): fill the read window instead of failing a partial read fs.read may answer short of what it was asked for before EOF, so the previous check turned a legitimate partial read into a spurious asset_changed. The loop mirrors the relay's readFullStreamChunk, which is not imported because it sits behind the relay dispatcher's module graph; only a read returning nothing is treated as truncation. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web-bundle): read the disconnect idiom with the shared predicate isClientDisconnectedError already exports exactly the check the catch needed, so the local error class goes away and the throw returns to the repo-wide idiom. The module doc now says asContractError is a total catch. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile-web-bundle): pin the four branches no test was holding Each one survived a mutation: the abort check before verification, the per-process manifest cache, the buildId component of the verdict key, and delete-at-zero in the admission map. The last two matter beyond hygiene — a verdict keyed by path alone carries a failed verdict onto the next build of index.html, and a map that never drops a key retains one pairing token per socket. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-18 01:04:27 -04:00
Neil	066b4951b9	fix(terminal): keep a split's real direction when the leaf set moves (#21294) ... resolveTerminalLayoutRoot discarded any known tree that did not cover the published leaf set exactly and rebuilt the tab as a flat chain with a guessed 'horizontal' direction, restacking side-by-side panes. The guess is then published, mirrored to every paired client, and written back over the real tree, so the direction is gone from disk. Prune a known tree to the leaves that survive and graft only the leaves no tree places, which is now the sole place a direction is invented and is still reported through onSynthesize.	2026-09-17 21:21:01 -07:00
Jinwoo Hong	ad4f26cdd4	feat(build): build, verify and package the mobile web bundle with every desktop release (OTA phase A, 2/5) (#21326) ... * feat(mobile-web): add the Phase A bootstrap web source A peer of src/ so the root workspace owns it and mobile's separate lockfile stays out of packaging. Four assets across four content types, enough to exercise multi-asset manifest handling rather than assume it. The page reads buildId from manifest.json at runtime: buildId hashes the asset list that index.html belongs to, so injecting it into a hashed asset would make that asset's hash depend on itself. Registered as a fourth typecheck project; without it the entry would be the only TypeScript in a release path that tsc never sees. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(build): build and verify the mobile web bundle from the root workspace Root esbuild over mobile-web/ into out/mobile-web/, content-addressed as assets/<sha256>.<ext> with index.html the only stable name. buildId is the sha256 of the canonical serialization of the sorted asset list, so it is a pure function of content and usable as a cache key with no further reasoning. The verifier builds twice into scratch dirs and compares: a timestamp, an absolute path, or an unstable ordering fails the build when someone introduces it, not the first time a phone gets a spurious cache miss. It also enforces the Phase A budget of 16 assets and 256 KiB, separate from the permanent contract ceiling. build:release does not call build:desktop, so build:mobile-web is wired into build:desktop, build:release, and build:release:parallel. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * feat(packaging): fail the release when the mobile web bundle is missing or stale electron-builder only warns about a missing input, so without a beforePack guard a release ships an app that advertises the bundle capability and then errors on every request. The hash check, not the existence check, is what catches a half-written or stale out/. The source tree is excluded from app.asar; out/mobile-web ships inside it under the existing out rules, exactly as out/web does. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile-web): narrow the manifest with `in` instead of a cast The changed-code casting gate rejects assertions, and `in` narrows the same untrusted JSON without one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web): move the bundle source under src/ so the root guard passes .github/scripts/check-root-directory-entries.mjs blocks any new top-level entry by name, so mobile-web/ could not live at the root. The source is excluded from app.asar by the existing '!src{,/**/*}' rule; the explicit '!src/mobile-web{,/**/*}' entry stays as a marker. out/mobile-web is unaffected and still ships under the out rules like out/web. No tsconfig includes src/**, so node, web, cli, and relay do not pick the tree up; it is registered as a knip entry so audit:dead-code does not call it unused. buildId is unchanged at 9d78435e: the builder hashes content, not paths. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(build): resolve the entry-script guard through pathToFileURL `file://${process.argv[1]}` never equals import.meta.url on Windows, where that url is file:///C:/... So the builder exited 0 having written nothing and the Windows packaging job failed later, at the guard, with no clue why. Every other script in config/scripts already uses pathToFileURL; this one now does too, via an exported predicate a posix runner can exercise with a win32 path. The verify script had no entry guard at all, so importing its budget constants ran the whole verification — including its process.exit — inside the test worker. It is now a function behind the same guard. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(ci): build the mobile web bundle in the PR package job That job assembles packaging inputs step by step instead of calling build:release, so the new beforePack guard hard-failed it. The census test added here is the oracle: it walks every workflow job that invokes electron-builder without --prepackaged (which short-circuits doPack before beforePack) and requires a bundle-producing script in the same job. It goes red on exactly pr.yml's package job when this step is removed. Ten jobs covered; the other nine already ran build:release, build:release:parallel, or build:desktop. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile-web): pin source line endings, because CRLF changes the buildId Every text byte under src/mobile-web is hashed into an asset digest and from there into buildId, so a CRLF checkout produces a different bundle id for the same commit: 91af2897 instead of 9d78435e. That would make a Windows-built desktop disagree with a mac-built one about which bundle a phone has cached. .gitattributes pins eol=lf for the text sources and -text for the PNG, matching the four trees already pinned for byte-hashing. The verify script asserts no source file carries a CR, so the build fails if the pin ever stops applying rather than silently shipping a second bundle identity. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(build): read the test's own path from import.meta.filename oxlint unicorn/prefer-import-meta-properties. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(test): census packaging jobs over raw workflow text, not re-serialized YAML yaml.stringify folds long lines, and in dev-channel-win-build.yml's build-win the fold landed between `electron-builder` and `--config`, so a real packaging job was invisible to the census: 11 jobs exist, the test saw 10. Slice each job's raw source by its parsed boundaries instead, and pin the inventory so a new packaging workflow has to be added here on purpose. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(build): assert the script chain the packaging census trusts The census only checks that a packaging job invokes one of ten build scripts; that those scripts still reach build:mobile-web was asserted nowhere, so a dropped link would leave every job looking covered while packaging failed at beforePack. Resolve each script for real, and pin pr.yml's hand-rolled step, since that job never calls build:release. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(build): realpath the entry path before the direct-invocation compare Node resolves symlinks in import.meta.url but not in argv[1], so `node /tmp/...` against a /private/tmp realpath compared two different strings: the builder and the verifier exited 0 having written and checked nothing. Same silent-success shape as the Windows file:// bug, so the fix sits next to it, with both seams injectable. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * style(mobile-web): format bootstrap.css with oxfmt It was the only tracked CSS failing oxfmt --check. The buildId is unchanged at 9d78435e8bb73c3341f833c20aaefbd7bfdfc414b68dadf87c1689d86728fe33, because esbuild's CSS minifier normalises the whitespace this touches before the asset is hashed. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(packaging): reject bundle files the manifest does not list The guard only walked the manifest, so a dropped assets/stale.js passed: assets are content-addressed, nothing ever overwrites a stale copy, and it would ship inside asar unreachable and unverified. Require every file under out/mobile-web to be the manifest or a listed asset. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(packaging): give beforePack an explicit mobile web bundle root The bundle guard read the repo's out/mobile-web unconditionally, so the two arch-aware packaging tests that call the real beforePack went red in the unit-test job, which never runs build:mobile-web. beforePack now takes the bundle root as a second parameter defaulting to out/mobile-web, which is what electron-builder gets, and those tests build a real bundle into a temp dir instead. The guard is neither skipped nor made tolerant of a missing bundle. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(packaging): census sees script-wrapped packers; dev verify reuses the guard The workflow census only matched a literal `electron-builder --config` line, so daemon-relocation-spike's `pnpm run build:unpack` (which packs and runs beforePack) was invisible to it. Jobs now count when any `pnpm run <script>` they invoke chains to electron-builder without --prepackaged; the spike joins the pinned list (12 jobs). verify-mobile-web-bundle.mjs re-implemented a weaker subset of the packaging guard (no safe-path check, no buildId recompute). It now calls assertMobileWebBundleBuilt, so a manifest edited after the build fails at `pnpm build:mobile-web` exactly as at beforePack. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb	2026-09-17 22:41:13 -04:00
Jinwoo Hong	a634bf9b49	test(bench): runtime-graph publication probe and optional CDP CPU throttle (#21107) ... * test(bench): count runtime-graph publications from main The build-provided `__orcaBenchmarkInstrumentation` is gone from the tree, so the typing bench could no longer report graph-publication counts at all. The renderer cannot supply them either: `window.api` is frozen by contextBridge, so `runtime.syncWindowGraph` is not wrappable. Count them where they land instead — main's `runtime:syncWindowGraph` invoke handler — behind ORCA_TYPING_BENCH_GRAPH_PROBE=1, and record the result in the bench report. Measured on an 870-worktree fixture: 21 publications over a 50 s metadata-only window versus ~1,205 with recurring OSC title/status traffic. The long-task fields ship unproven: an injected 250 ms renderer busy-wait produced zero entries even though `longtask` is in `supportedEntryTypes`, so their zeros mean "oracle unverified", not "no long task". The self-test knob exists to make that falsifiable, and the file says so; per-publication build time still needs a separate --cpu-profile run. * test(bench): optional CDP CPU throttle around the typing window * test(bench): report the throttle that ran and the long task the self-test caused Two ways the bench could misreport its own conditions. `cpuThrottleRate` was the requested rate, written into every report, but only two of the three scenarios wrapped their typing window in the throttle — a `--cpu-throttle 4` visible-split run claimed a 4x throttle it never applied. Recording the rate per scenario would have made the report honest; it would also have left one scenario silently ignoring the flag, and a fourth scenario would inherit the same omission. So both: every scenario now goes through one `measureTypingWindow` helper, and the value it returns is the rate the throttle actually applied. `writeBenchReport` takes that composite instead of a bare measurement, so a scenario cannot produce a report without saying what it ran under. Unthrottled runs are unchanged — rate 1 still opens no CDP session. `selfTestLongTaskMs` took the *earliest* long task starting before a cutoff captured after the busy-wait. The observer has been live since probe start, so any unrelated long task from fixture setup satisfied it — the field whose whole job is to prove the oracle is live was the easiest one to fake. The busy-wait now reports its own renderer-clock bounds and the matching entry is the one containing their midpoint: main-thread tasks never overlap, so at most one can, and it is the task the busy-wait ran in. That entry is then withheld from `longTasks`, `longestLongTasks`, and `longTasksAroundPublication`, which had been counting the oracle's injected 250 ms as workload. A zero still means "oracle unproven" — it now also means it honestly. * test(bench): stop the graph probe when the typing run throws * test(e2e): drain queued long-task records before the probe disconnects	2026-09-17 17:32:20 -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
Neil	ea01cd0ccd	fix(windows): reject a node-pty addon that predates the MSYS breakaway denial (#20047) ... * docs(windows): record the measured MSYS job-breakaway mechanism The per-PTY job already denies JOB_OBJECT_LIMIT_BREAKAWAY_OK for Cygwin/MSYS shells (#19068), but nothing records why, and a conpty.node built before that commit fails windows-msys-job.win32.test.ts in a way that reads as a source defect. Measured on a real Windows 11 host: both the plain and the exec- replacement Git Bash shapes leak, the escape is the MSYS runtime's own spawn/exec (fork keeps membership), and a single-variable A/B on usesCygwinRuntime flips the result 0/2 -> 4/4. Also names the gap the failure hid behind: node-pty-job-ownership.cjs asserts symbol presence, which cannot distinguish patch revisions. * fix(windows): reject a node-pty addon that predates the MSYS breakaway denial The native-runtime gate asserted only that terminateJob, listJobProcessIds and assignCurrentProcessToJob were exported. All three predate the Cygwin/MSYS breakaway denial, so an addon built before it passes every gate, isPtyJobOwnershipAvailable() returns true, and windows-pty-job.win32.test.ts passes 6/6 -- while every Git Bash child is created outside its pane's job and survives terminatePtyJob. Read the resolved .node and require the wide msys-2.0.dll literal that usesCygwinRuntime holds, the way stagedRelayAddonIsUnpatched() already tells a patched windows-process-tree addon from a published one. An addon the caller cannot name is refused rather than skipped: a gate that cannot see its subject is not a gate. Verified against real binaries on a Windows 11 host: the shared checkout's pre-#19068 build errors, a build from current patched source passes, a missing path errors. Also closes the cross-host packaging skip. The export half has to load the addon so it cannot run when the packaging host is not the target, which is how a Windows release built elsewhere could ship this. The marker is a file read and needs neither; an unrecognised layout warns rather than fails a release that was packaging fine. * fix(windows): check the MSYS breakaway denial on the rebuild path too The Electron probe carried the marker check, but it lives inside probeElectronNativeModules, which returns early whenever the Electron package binary is unusable. Covered by another path is not this path checks -- and the defect this whole change closes was a gate that looked like it checked. Reading the binary needs neither a loadable Electron nor an executable target arch, so assert it after the rebuild, beside the windows-process-tree assertion that exists for the same reason: this is the addon copied into the packaged app. Absent warns (a cross-platform rebuild need not leave a win32 addon on this disk); present and unmarked is fatal. The fixtures now write a real addon file, because the gate reads the binary it was told about rather than trusting the exports. Verified against the two real binaries measured on the Windows host: the pre-#19068 build fails this path, the build from current patched source passes. * fix(windows): check the marker on every ConPTY path the packaged app can load The packaged marker check read one hard-coded path, `build/Release/conpty.node`, and warned when it was absent. `loadNativeModule` tries `build/Release`, then `build/Debug`, then `prebuilds/win32-<arch>`, swallowing each failure, and `prunePackagedNodePty` drops the published prebuild only when a same-arch `build/Release` exists to replace it. So the two packages the check was added for were the two it could not see: - cross-host: no host but Windows can build conpty.node, so there is no `build/Release` and the prebuild is what ships. The check warned and returned. - cross-arch: `build/Release` is the packaging host's own arch, patched and marked, so the check printed OK -- while the target app cannot load it and falls through to the unmarked prebuild underneath. Measured, not assumed: both published Windows prebuilds in the node-pty tarball contain neither `msys-2.0.dll` nor `cygwin1.dll` in any encoding. They are the binary that leaks every MSYS pane child out of its job. It now sweeps every candidate present for the *target* arch and refuses a package with no candidate at all, which is a package with no ConPTY backend rather than a layout to shrug at. It runs for every Windows slice instead of only the branch the export check skips, so deleting the export check cannot silently take it too. A stale source build keeps the rebuild advice; the prebuild gets the advice that actually works, which is to package the slice on a Windows host of that arch. Also: the marker constant was re-typed in four places and was tied to the C++ literal that produces it by nothing at all, so editing the patch would have left a gate that fails every correctly rebuilt addon and tells the developer to do the one thing that cannot help. The fixtures now take the constant from the gate, and a test asserts the patch still adds `L"msys-2.0.dll"` to conpty.cc. And the rebuild path treated a missing addon as a warning even on the host that will run the install, where node-pty would fall through to that same prebuild. The verdict is now a value, so it is tested without a platform gate. * fix(windows): resolve the packaged ConPTY the way its loader does Sweeping every candidate and demanding the marker on all of them was wrong in the one case it was meant to make safe. `beforeBuild` runs `rebuild-native-deps.mjs --platform=win32 --arch=<target>`, so a cross-arch slice normally does get a patched `build/Release` for the target; `prunePackagedNodePty` keeps the prebuild anyway because its guard is `electronArch === process.arch` rather than the arch of the binary. That package is correct and its leftover prebuild is never reached, and the sweep failed it -- telling whoever ran it to package on a Windows arm64 host, which is both the wrong remedy and one no runner here can offer. Presence cannot separate that package from the one whose cross-arch rebuild quietly emitted the host's architecture, because the only difference is the arch of `build/Release`. So the gate now resolves the addon the way `loadNativeModule` does -- first candidate whose PE `IMAGE_FILE_HEADER.Machine` matches the target, walking root-then-lib for each layout in node-pty's own order -- and checks the marker on the one that will actually run. A package with no candidate, or none of the target's architecture, is refused: it has no ConPTY backend either way, and the second is exactly what a silently host-arch cross-build looks like. The PE machine reader already existed, privately, in the relay addon builder that needed the same "a cross-build cannot silently emit host arch" guarantee. It is now shared rather than copied. Two seams were unreachable from anything but Windows, so nothing tested them: - the afterPack hook's win32 block was an inline if/else that only a source-text assertion could inspect, and that assertion could not tell the difference between the check running and the check being wrapped in `try {} catch {}`. It is now `verifyPackagedWindowsNodePty`, and "the marker check runs even where the export check cannot" is four spied assertions instead of a string match. - the rebuild path's verdict read `process` directly, so the branch that fires only on the host being rebuilt for was dead on every other host. It now takes the host as arguments, and the fs checks, the warning and the failure are all exercised from macOS. Fixtures write a real PE header rather than `MZ fake addon`, since the gate now reads one. The machine table is pinned to the documented IMAGE_FILE_MACHINE values, because every fixture builds its header from that table and a table wrong in both entries would otherwise agree with itself. * fix(windows): say why the packaged ConPTY fell back, not just that it did The previous commit resolved the addon by architecture but still had one message for every way the resolution could land on the published prebuild. Those ways want opposite remedies, and the one it printed was the remedy the commit before it had just called wrong: - no source build in the package at all — the slice has to be built somewhere that can build node-pty for the target arch. - a source build that is there but is the packaging host's architecture, because the cross-arch rebuild did not honour `--arch` — re-running that rebuild is the fix, and "package on a Windows arm64 host" is neither necessary nor possible. The second is the common one, since node-pty publishes a prebuild for both Windows arches and prune keeps the target's on every cross-arch package. So the old text fired mostly on the case it described least. It now reports which source builds were skipped and the machine field each carried, and names the rebuild command. "Nothing the target can load" had the same problem in reverse: a zero-length or truncated `conpty.node` got a cross-architecture diagnosis. Every candidate is now named with what was actually read, including "not a PE image". The rebuild path asserts the architecture too. A rebuild that ignored `--arch` was otherwise only visible at packaging, two steps from the command that fixes it. Arches with no known machine value are left unjudged rather than guessed at. Two things the extraction broke or nearly broke, both found by mutation: - the shared PE reader answers `null` where the relay builder's private copy returned a number, which would have turned its "node-gyp ignored --arch" error into a `TypeError`. Both callers now go through `describePeMachine`. - the rebuild fixtures stage a script's co-located modules by walking its imports, and the walker only understood `from '...'` — so the gate's new `require('./windows-pe-machine.cjs')` was left behind and every subprocess test failed with a resolution error, which is the exact failure its own comment warns about. It now follows `require` and bare side-effect `import` as well, and has tests; the fixture stages the gate by walking it rather than by naming one file. Fixtures write real PE headers through one shared builder instead of three hand-rolled ones. * fix(windows): run the node-pty addon gates on the Windows job that can `rebuild-native-deps-node-pty.test.mjs` carries four `skipIf(platform !== 'win32')` tests. The full suite runs on ubuntu, and the Windows PR job runs an explicit file list that never named this file -- so those tests were skipped on Linux and never reached anywhere else. Three of them predate this branch. The Windows job is added the four node-pty addon suites plus the module-walker one; the comment above that list already says why it is the right place, which is that the addon assertions only hold once natives have been rebuilt. Running the path-joining suites there also covers the separator this gate's candidate list is built from. The rest is round-three review: - the rebuild-time arch assertion told a reader "node-gyp did not honour --arch" about a file that was not a PE image at all, which is a truncated or quarantined artifact and a different command to run. The two now read differently, and neither claims the other's cause. Same fix the packaged gate had one commit ago, in the place that had not had it yet. - the missing-addon error said node-pty "would load" a prebuild without checking it is there. It says "fall through to" now, which is true either way. - `isLoadableByArch` had no caller left once the packaged gate started needing the raw machine field for its message. Removed rather than kept warm. - each candidate's header is read once instead of up to three times. - the module walker's comment claimed every shape that reaches a co-located module; it does not follow `projectRequire`/`requireLocal`, and it must not -- those specifiers resolve against the project root, so following one stages the wrong path and the copy fails. Proven by trying: widening the pattern to require-shaped names broke nine tests on `projectRequire('./config/scripts/...')`. The comment now says what it follows and why it stops there. - a new test resolved a file URL with `.pathname`, which keeps the drive-letter slash on Windows -- the very job this commit adds it to. * docs(windows): put the superseded export-only gate in the past tense It describes what used to pass a broken addon, so present tense reads as a description of the gate the same document then explains replacing it. * fix(windows): repair what running the node-pty suites on Windows exposed Putting these files on the Windows job turned four assertions red on the first run. Three of them were in tests that carried `skipIf(platform !== 'win32')` and had therefore never executed anywhere, on any branch. - `writeFakeElectronRebuild` emitted the `windows-process-tree` addon a real rebuild leaves but never node-pty's, so every Windows test of the rebuild path ran against a tree no real rebuild can produce: node-pty "rebuilt" with nothing in `build/Release`. The new same-host check reads that state correctly and said so. The fake rebuild now writes `build/Release/conpty.node` when it was asked to rebuild node-pty for win32, with the marker and the target machine. - `mkTempProject` never staged `windows-process-tree-creation-time.cjs`. The rebuild script reaches it through `projectRequire`, which resolves against the project root, so the module walker cannot follow it and must not try. Staged by name, with a comment saying which of the two it is. Without it the windows-process-tree probe failed to load its own checker and the module joined `modulesToRebuild`, which is the second and third red assertion. - the two `nodePtyAddonPath` cases compared against a literal POSIX string. `resolve` returns a drive letter and backslashes on Windows, so they could only ever pass off it. Built from segments now, which still pins the `..` traversal that is the point of the test. Verified on macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime -- 109 passed, 6 skipped. The 6 are the Windows-gated rebuild tests, which is the job this change is aimed at; Windows CI is the arbiter. * fix(windows): give the packaged fallback a third verdict, for a file that is no image The packaged gate had two remedies for landing on the published prebuild and picked between them on `!prebuilt`, which puts a truncated, empty or quarantined `build/Release/conpty.node` in the cross-arch bucket: "the source build beside it is the wrong architecture ... re-run with --arch". It is not the wrong architecture, it is not an architecture, and `--arch` is not the command. The rebuild-path gate was split for exactly this a commit ago; this is the same split in the place that had not had it. Also from review of the settled state: - the stale-source-build branch ended in a call that happened to throw, so a reader could not see it was terminal and the file was read twice to get there. The verdict is now an Error the caller throws, built once from the read it already did, and shared with `assertCygwinBreakawayDenied` rather than copied. - four injection seams had no consumer in production or in tests (`deniesBreakaway`, `peMachine`, and `exists`/`peMachine` on the rebuild verdict). An unused seam is a way for the tested path and the real one to drift apart; the tests drive both with real files. Removed. - the loader table existed in a docblock and in the reference doc, already disagreeing about row four. The docblock cites the doc now. - `peImage` stamped machine `0x0000` for an arch it had no value for, because `writeUInt16LE(undefined)` coerces to zero. A fixture that quietly invents the field the gates read is the same species of silent lie the gates exist to catch; it throws, and a test holds it to that. - a test named for refusing an unreadable candidate asserted only that something threw. Renamed to what it proves. * fix(windows): make the rebuild fixtures represent a tree that can exist Second round of what running these suites on Windows exposed. The module the walker could not stage is now staged, so the probe reached its own checker and the real reasons surfaced: - `writeFakeWindowsProcessTree` exported `{}`. The creation-time gate reads `supportedProcessDataFlags` off the addon and calls its absence "the tarball prebuilt, not a build of the patched source" — correctly. The fixture predates that gate and, being Windows-only, never met it. The healthy fake now reports the flag, taken from the gate's own constant. Two tests were failing on this, the second only because the module then joined `modulesToRebuild`. - `rebuilds a loadable ConPTY native that lacks Orca job ownership` asked for a node-pty rebuild in a tree where node-pty had none of the payload its package ships. It gets `writeFakeNodePtyConptyPayload` like its two siblings. I also tried making the fake rebuild emit `build/Release/conpty.node` the way a real one does, and backed it out: `restoreNodePtyWindowsConptyRuntime` keys off that file and then reads `third_party/conpty`, so emitting it in a tree without the package payload turns one honest gap into an ENOENT two steps away. The payload fixture is where "node-pty has its addon" belongs. macOS: ensure-native-runtime-job-ownership, verify-packaged-node-pty-job-ownership, windows-pe-machine, script-module-dependencies, rebuild-native-deps-node-pty, rebuild-native-deps, rebuild-native-deps-windows-process-tree, ensure-native-runtime — 112 passed, 6 skipped. The 6 are the Windows-gated rebuild tests; Windows CI is the arbiter and is why they are on that job now. * fix(windows): register the node-pty addon suites in the scope list too Putting the five suites in the Windows lane's vitest argv gets them run once the job starts; `WINDOWS_PACKAGE_TESTS` in `pr-code-change-scope.mjs` is what decides whether the job starts at all. Only the argv was updated, so a PR touching just `rebuild-native-deps-node-pty.test.mjs` would not have started the Windows job, and its four Windows-only cases — including the same-host-absent one added here — would have run on no machine for that PR. Exactly the shape of gap this branch is about. Both lists now name all five, and `windows-pe-machine`, `windows-pe-image-fixture` and `script-module-dependencies` join `NATIVE_RUNTIME_PREFIXES` so a change to the modules themselves starts it too. `win32-test-lane-registration.test.mjs` exists to catch precisely this and did not, because its matcher only recognises suite-level gates (`describe.runIf` / `describe.skipIf`) and a `.win32.` filename. These tests gate per `it`. Widening it is not this branch's change to make: about thirty files across the repo carry per-`it` Windows gates and are unregistered, so the ratchet would move far beyond node-pty. Flagged rather than done. Message repairs from the same review: - the non-PE arm of the rebuild-time arch error read "... is not a PE image, so nothing can load it, so node-pty would fall back ...". The shared consequence clause already opens with ", so". - the no-source-build packaging error ended "Package this Windows slice on such a host", which is wrong advice for the case where the host IS such a host and the rebuild simply left nothing — reachable when the artifact is removed before prune runs. It now names both readings and points at the beforeBuild output. - the relay-addon builder blamed `--arch` for a build output that is not a PE at all, the same guess the node-pty gate was taught to stop making. - the patch-drift assertion was a bare `toBe(true)`, so a real drift read as "expected false to be true". It now names the two things that can have drifted and what happens until they agree.	2026-09-16 22:23:30 -07:00
Brennan Benson	fbe7b194b8	fix(quality-gate): let the changed-code gate see the focused import plugins (#20912) ... import/no-duplicates was reachable only through the repo-wide CI audit, so an author's first signal was a red static analysis job after push.	2026-09-16 20:54:27 -07: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	12d744f253	fix(skills): keep computer-use off filesystem and shell tasks (#21069) ... * fix(skills): keep computer-use off filesystem and shell tasks STA-7615: "On my desktop create a folder" was matching computer-use because discovery copy said OS/window-level and neighboring skills advertised desktop UI. Scope the trigger to visible GUI with no CLI path, and exclude files/folders/git/shell. * fix(skills): prefer programmatic paths over computer-use State the last-resort rule in discovery copy instead of enumerating files/folders/git/shell. computer-use prefers shell, filesystem, git, HTTP, CLIs, and Playwright/CDP; neighboring skills route to Computer Use only when a visible window needs GUI control those cannot do. * fix(skills): stop advertising computer-use from orchestration Orchestration coordinates workers; it does not drive a GUI. Drop Computer Use and Playwright/embedded-browser routing from its discovery description so those tools are not pulled in from a coordination skill. * fix(skills): drop Playwright from orca-cli discovery orca-cli should not prescribe Playwright or CDP. Those tools may not be installed, and page automation is not this skill's job. * fix(skills): drop the page-only ban from computer-use discovery Page automation is a preference, not a prohibition. If Playwright or CDP is not available, a visible browser window is valid Computer Use. Keep the hard split for Orca's embedded browser (`orca-cli`) only.	2026-09-16 15:43:11 -04:00
Jinwoo Hong	bdb18003e0	test: add accumulated-workspace terminal typing reproduction (#20934) ... * test: reproduce accumulated-workspace typing latency through real PTYs * test: make the bench harness self-checks falsifiable Review found four assertions that could not fail and one fixture gap: - `missingPtyArrivalCount`/`missingEchoCount` were hardcoded `0` and `validateExpectedSeqs` throws before them, so every assertion on them was vacuous and every report read `0`. The throw is the real guard and is already covered; drop the vestigial fields. - An absent status controller returned an all-zero result, which satisfied its own accepted-equals-generated equality. Assert presence first. - The byte-pacing control had only an upper bound, so a generator emitting no stream bytes passed. Add the lower bound. - `lineageEvery: 1` built zero lineage: no ordinal satisfies `% 1 === 1`. Offset the interval and cover the densest setting. - The documented control command never set ORCA_TYPING_BENCH, so it skipped instead of running.	2026-09-16 13:10:46 -04:00
Brennan Benson	170ebce1f2	fix(ci): run static analysis for every tree the repo-wide audits scan (#20918) ... A mobile-only diff is desktop-irrelevant, so should_run was false and every PR check skipped -- including the audits that do lint mobile/. The violation then landed on main and failed the same gate on every later PR's merge ref. Derive the trigger from the audit commands' own scan roots so the two cannot drift.	2026-09-16 01:13:06 -07:00
Neil	d62328aa4d	fix(codex): remove redundant Windows hook launcher for Unicode profiles (#20952) ... * fix(codex): reuse the Windows hook shell for Unicode profile paths * test(codex): register Unicode hook tests in Windows CI * test(codex): pin trust hash replacement during Windows upgrade * test(codex): retry transient Windows teardown locks	2026-09-16 00:30:52 -07:00
Jinjing	47bb473ec6	Remove agent map from dashboard popout (#20929) ... The agent map view was not functional and its components have been removed entirely. The dashboard popout now only supports the kanban board view, with all map-related code, utilities, types, and translations cleaned up accordingly.	2026-09-15 21:55:06 -07:00
Neil	13ba649c22	fix(terminal): let a runtime-created Windows terminal BE the requested shell (#20825) ... * fix(terminal): let a runtime-created Windows terminal BE the requested shell `orca terminal create --environment <windows-host> --command 'cmd.exe'` never created a cmd terminal. `--command` is text the provider TYPES into whatever shell it spawned, so the PTY stayed the host's default shell with cmd running inside it. Captured on `awin`, whose default is Git Bash: $ orca terminal create --environment awin --command 'cmd.exe' --json $ orca terminal send --environment awin --terminal term_10656cf7... \ --text exit --enter $ orca terminal read --environment awin --terminal term_10656cf7... --screen neil@awin MINGW64 ~/orca/orca ((30f820708f...)) $ cmd.exe Microsoft Windows [Version 10.0.26200.9445] C:\Users\neil\orca\orca>exit neil@awin MINGW64 ~/orca/orca ((30f820708f...)) $ The handle is alive the whole time and `terminal list` shows one healthy terminal, because the PTY never changed — so the only symptom is that the caller's terminal is now a shell it never asked for, and every later `send` is quoted for the wrong one. On `win-lowspec` (default pwsh) the same create lands cmd inside PowerShell. Root cause ---------- There are two spawn preflights and they are twins: - `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns, i.e. a terminal tab opened in the app. - `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: the CLI's `terminal.create`, headless `orca serve`, and every paired remote environment. Only the IPC twin read the caller's requested shell. The runtime twin passed a literal `requestedShellOverride: undefined`, so a runtime-created terminal on Windows could only ever be the host default. Everything downstream of that point — `spawn-options`, the daemon, `resolvePtyShellOverride` in the relay, `local-pty-launch-plan` — already honoured `shellOverride`; nothing upstream could supply one. Change ------ - Thread `shellOverride` through the runtime lane: `RuntimePtySpawnArgs` -> runtime `spawn-preflight` -> `RuntimePtyController.spawn` -> `TerminalCreateOptions` -> the `terminal.create` RPC's new `shell` param -> `orca terminal create --shell`. - Thread it through the renderer-backed lane too (`createDesktopTerminal` -> `terminal:requestTabCreate` -> `store.createTab`), so `--shell --focus` is not silently dropped on a local Windows app. - An agent launch quotes its startup command for the shell it will actually run in, so a requested shell now owns the startup-shell family instead of the global `terminalWindowsShell` setting. - Lift the relay's `ALLOWED_WINDOWS_SHELL_OVERRIDES` into `isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts` (membership unchanged) so the CLI, the zod param schema, and the relay refuse the same names. `--shell` therefore cannot carry a path or a command line into `pty.spawn`; only allowlisted bare shell names pass. - Gate on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY`. An older host strips the unknown `shell` param and answers with a healthy terminal running its default shell — a reply indistinguishable from success — so the CLI refuses before creating anything rather than creating the wrong shell quietly. `--shell` stays Windows-only; macOS and Linux hosts spawn the login shell and the relay drops the value off win32 rather than honouring it half-way. A WSL project runtime still outranks it, unchanged. Tests ----- - `pty-spawn-shell-override-parity.test.ts` pins both preflights against the exact drift that caused this (verified failing with the fix reverted). - `createTerminal` passes `shellOverride` to `ptyController.spawn` with no startup command. - CLI: sends `shell`, refuses a shell the host cannot spawn, and refuses a host without the capability — in both refusals without making the round trip. - Allowlist and `terminal.create` schema accept/refuse cases, including paths and appended arguments. * fix(terminal): refuse a requested shell the execution host cannot apply The first commit made `--shell` reach the spawn, but only a LOCAL win32 execution host applies it: `spawn-options` gates the override on `process.platform === 'win32' && !args.connectionId`. So `--shell cmd.exe` against an SSH-routed worktree, or against a macOS/Linux host, still returned a healthy terminal running that host's default shell — the same indistinguishable-from-success reply the capability gate exists to prevent, one layer down. Refuse instead, before anything spawns. The check sits at the top of `resolveAgentTerminalCreateOptions`, which every create lane funnels through, so neither lane has to remember it; the desktop lane additionally refuses a worktree-less create, which has no execution host to resolve a shell on. An SSH host's platform and installed shells are not visible to this runtime, and a POSIX host has no Windows shell to pick. Neither can honour the request, and saying so is the whole point of the flag. Docs and the CLI spec now say "refused", not "ignored". * fix(terminal): refuse a shell that contradicts the project execution runtime `resolveLocalWindowsTerminalRuntimeOptions` does not merely rank the project's execution runtime above a per-terminal pick -- it REWRITES the pick, in both directions, and says nothing: - a WSL project forces `wsl.exe`, discarding `--shell cmd.exe`; - a Windows-host project discards a WSL name and falls back to `COMSPEC` (`getHostShellForProjectRuntime`), so `--shell wsl.exe` spawns cmd. That is the common case, not an edge: `resolveProjectExecutionRuntime` resolves `windows-host` for every project that is not WSL, while a repo belonging to no project honours `wsl.exe` -- so the same flag behaved differently depending on whether the repo was in a project. Either rewrite returns a healthy terminal running a shell the caller did not ask for, which is the failure `--shell` exists to remove. It also split an agent launch's quoting from the shell that receives it. The previous commit made the startup-shell family follow the REQUESTED shell, so `--shell wsl.exe --command codex` on a Windows-host project typed POSIX-quoted launch args into cmd. Refusing the contradiction removes that case rather than papering over it. Refuse instead, alongside the SSH and non-Windows refusals, from the same `resolveAgentTerminalCreateOptions` seam every create lane funnels through. Also from review: - the allowlist test looped the list against itself; spell the members out. - the runtime spec case claimed to prove the pty's shell when it asserts the controller received the field; name it for what it checks. Reported by an adversarial review of the branch. * fix(terminal): canonicalize --shell and refuse a WSL-path rewrite Review of the --shell create path turned up two ways the terminal could still end up being a shell the caller never asked for -- the exact failure --shell exists to remove. Bare and mixed-case spellings passed the allowlist but reached consumers that exact-match the canonical name: resolveWindowsShellStartupFamily classified `cmd` as the PowerShell family, resolveWindowsShellLaunchArgs fell through to empty shellArgs (no `chcp 65001`, no OSC 133 bootstrap that Windows foreground status depends on), and resolveWindowsGitBashShellPath compares case-sensitively so `Git-Bash` spawned a literal `Git-Bash`. The allowlist is now one canonical-name map and terminal.create canonicalizes on parse, so the spawn path only ever sees `.exe` spellings. `pwsh` and `powershell` stay distinct binaries. A `\\wsl$\<distro>\...` cwd made the providers force wsl.exe regardless of the request, and terminalShellOverrideRefusal only inspected the project runtime -- undefined for a folder workspace with no project. Refuse on the resolved cwd and the workspace path, judging what the PTY actually gets. Also: the capability gate reported an unreachable host as too old rather than unavailable; the SSH CLI shim dropped capabilities from status, so --shell there blamed the host version instead of naming SSH; and --shell had no help entry, rendering bare in `orca terminal create --help`. Adding that entry crossed help.ts's max-lines cap, so the flag table moved to flag-help-text.ts rather than suppressing the rule. Adds a behavioural test for the runtime preflight (the one-line fix was pinned only by a source-text scan), plus coverage for the startup-command quoting family, the no-workspace refusal, and the WSL-path refusal. * fix(build): keep tests out of the RPC params catalog bundle The catalog walk under methods/ already skips *.test.ts, but the contract directory glob took every .ts. terminal-create-shell-param.test.ts is the first test to live there, so the bundle pulled vitest into a CJS build and the generator threw on require(). Same exclusion, same reason.	2026-09-15 16:34:16 -07: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
Neil	bfdec26352	fix(lint): enable anti-slop/no-object-parameters (#20781) ... The rule rejects the broad `object` type on any function input (declarations, expressions, arrows, methods, call/construct signatures, function types), plus local aliases and unions that resolve to `object`. `object` accepts every non-primitive while exposing no properties, so it documents nothing and pushes callers into assertions at the boundary. Fixes all 185 violations across src, config, tests and mobile, and flips the rule from "off" to "error" in config/oxlint-anti-slop.json. Approach: replace each `object` input with the type its owner already has. Most sites took an existing domain type or a type-only import (36 added); 40 new aliases name shapes that had none. Where a value is genuinely only compared by reference, it gets a named identity token instead of a shape -- `Record<string, never>`, the built-in `WeakKey`, or a `unique symbol` brand, matching the branding already used in src/shared. Same treatment for WeakMap and Map key parameters. Two `as unknown as` casts became unnecessary once the parameter carried a real type and were removed; no new casts were added. Suppressions added: none. No `oxlint-disable` for this rule anywhere, and no max-lines disable or per-file bump. Three files sat exactly at their max-lines cap, so the added type imports were made line-neutral rather than suppressed: - src/main/ipc/browser.ts exports the existing guest-registration args type (renamed BrowserGuestArgs) so browser.test.ts reuses it on one line. - pane-scroll.ts takes TerminalScrollIntentTarget through the existing pane-manager-types import via a type-only re-export. - direct-rpc-client.ts drops the identity parameter entirely: the session check moved into the sendProbe callback that owns the token. Verified: anti-slop config reports zero violations over src config tests mobile; run-typecheck-projects-in-parallel exits 0; 144 affected test files pass (1749 tests); oxlint and oxfmt clean on all changed files. Mobile has no runnable test/typecheck target in this worktree (expo is not installed), so its 6 files were typechecked against a standalone config and diffed against the base branch -- error sets are byte-identical, including test files.	2026-09-15 01:59:58 -07:00
Neil	f7b2736d6d	fix(worktree): block removal when the archive hook fails (#20153) ... * fix(worktree): block removal when the archive hook fails A repo's orca.yaml archive hook is the user's last chance to save work off a checkout Orca is about to delete. A failed hook was logged as advisory and stepped over, so the removal went ahead with nothing archived — and the caller could still be told it succeeded. The hook is now a blocking precondition, evaluated while the checkout, its Git registration, its agents and Orca's ownership evidence are all still intact: it sits ahead of the registration re-read, the lock/dirty preflights, stopPtys() and removeWorktree in every orchestrator that runs it. Failure is typed (worktree_archive_hook_failed) and carries the worktree path, outcome, exit code where one was observed, and the hook's output. unverifiable stays distinct from exited, so loss of contact is never read as a pass. The waiver rides its own field at every layer and is never implied by --force, which already carries the PTY-stop waiver; when used, the waived failure comes back on result.archiveHookOverride rather than being swallowed. worktree.archive-failure-blocking.v1 is advertised so an integration can tell "accepts --run-hooks" from "safely propagates a failing hook" without risking the data loss to find out. The runtime's SSH path cannot run a hook at all, so rather than delete with the archive step silently skipped it refuses — waivable like every other refusal here. #18563 retires that gate by making the path run the hook for real. Stacked on #20559, which makes a timed-out hook report honestly; without it a hook that traps SIGTERM and exits 0 would defeat this gate. Fixes #19334 * fix(worktree): close the skip-confirm dead end and the client/hook timeout gap Four review findings on the gate. A retry from the failure toast could fail for a DIFFERENT reason than the one the user had just answered, and that second failure got a bare toast with no buttons. With skipDeleteWorktreeConfirm set, the delete helpers pass no force, so waiving a failed archive hook on a dirty checkout landed on the dirty preflight and stopped there. Retry failures now re-enter the same failure toast, so every retry stays as actionable as the first attempt. Third instance of this class. The renderer gave worktree.rm a 60s budget while an archive hook may run for 120s. A hook that took 90s and succeeded timed the client out and reported failure while the host went on to delete — telling the user their delete failed and their checkout was gone. The budget is now derived from the hook's, and only when a hook can run. The SSH fail-open is logged rather than silent, and the capability's doc comment scopes what it claims: a hook that RUNS and fails cannot delete the checkout; it is not a promise the hook was found. The SSH owner-resolution test now reads a real remote orca.yaml through a stubbed provider and asserts the returned script is the remote one. It previously stopped at the lookup key, which is the coverage that let this path break twice. It fails against the row-only resolution. * fix(worktree): name a signalled hook exit, and state why prunable cleanup skips the gate Two things the rebase onto #20617 and #20576 surfaced, both found by rerunning the real-repo harness rather than by reading the diff. - #20617 added a registration-cleanup branch that returns before the archive gate. That ordering is correct — both of its arms describe a row with no checkout behind it, so there is nothing to archive and running the hook would fail on the missing cwd — but the gate's ordering invariant is documented, so the exception should be too. - A signalled hook reported `Command failed with exit code null.`, which reads as a reporting glitch rather than the `unverifiable` verdict it is about to produce. It now says the command was terminated without reporting an exit code. Introduced by #20576; the withheld `exitCode` itself was always right. Fixes #19334	2026-09-15 01:19:32 -07:00
Neil	37a5b278b3	test(package): reject an Electron install takeover by exact command (#20799) ... * test(package): reject an Electron install takeover by exact command CodeRabbit was right about #20787. Replacing the pinned postinstall string with a /electron/i keyword check was wrong in both directions, verified: rebuild-native-deps.mjs && rebuild-native-deps.mjs PASSED (should fail) rebuild-native-deps.mjs && check-electron-version FAILED (should pass) The owner's own path contains no "electron", so duplicating it slipped through -- the one case the contract is named for. And a substring match rejects any later step that merely mentions Electron, which is the same over-tightness that broke every open PR in the first place, relocated. Later steps are now checked against the exact owned command plus the known Electron install commands. A second case pins the rejections themselves, because reading the real postinstall cannot show a bad chain would be caught -- that is how #20787 shipped with a guard that did not guard. Split into its own file rather than adding a max-lines disable (AGENTS.md). * test(package): match install commands as tokens and cover the rebuild:electron alias Both review comments were right, verified by running them: && check-install-app-deps-version.mjs rejected by substring match (should pass) && pnpm run rebuild:electron slipped through (should fail) package.json:101 aliases rebuild:electron to the owned script, so invoking it is the same takeover. Matching is now token-based with the owned command still checked as a phrase, and both cases are pinned.	2026-09-15 01:10:35 -07:00
Neil	22ce8d69a1	fix(lint): enable anti-slop/no-module-mocking (#20783) ... The rule rejects `vi.mock` / `vi.doMock` / `vi.unstable_mockModule` and the `jest` equivalents, on the argument that a test which rewrites the module graph asserts against a stand-in the production code never sees. It is already off for `**/*.test.{ts,tsx}`, `**/*.spec.{ts,tsx}`, `tests/**` and `**/__mocks__/**` via the existing override in config/oxlint-anti-slop.json; that override is unchanged here. What the rule actually catches is module mocking that has drifted out of a spec and into a first-party `.ts` support module, where nothing marks it as test-only. 73 violations at baseline, all of them in test-support code. 9 were relocated back into spec files the override already exempts; the remaining 64 sit in 10 files that are test-only but do not match the override globs, and carry a file-level disable naming the rule and the reason. Relocated: - terminal-hydration-store-test-bootstrap.ts: the sonner / sync-runtime-graph / pty-transport `vi.mock` calls moved into the two specs that import it (terminals-hydration-canonical-rows, terminals-hydration-canonical-pty-overlap). Vitest hoists `vi.mock` inside a test file, so registration is strictly earlier than the previous module-eval-time call; the bootstrap keeps only the preload API proxy. Both importers were updated. - ipc-events-ssh-authority-test-fixtures.ts: the 6 direct-ssh `vi.doMock` calls moved into useIpcEvents-agent-status-ssh-authority.test.ts as a local `stubDirectSshModules()` helper, which also de-duplicates the three copies the spec already had inline. The fixture now returns the store state and coordinator doubles it builds, typed via the exported DirectSshReconnectCoordinatorDouble. Suppressed, with justification (each is `/* oxlint-disable anti-slop/no-module-mocking -- ... */`, rule named, no blanket disable): - config/scripts/headless-serve-shutdown-matrix.test.mjs (1) - a genuine Vitest spec that the override misses only because its globs say {ts,tsx}. The script under test is a top-level CLI module; the alternative is spawning real docker. - src/main/codex-accounts/runtime-home-service-test-harness.ts (1) - stubs one probe predicate in ../pty/shell-startup-env, imported directly by several main-process readers; 17 specs share it. - src/main/computer/desktop-script-provider-test-harness.ts (2) - stubs child_process/fs-promises for a provider that shells out; 8 specs share it. - src/main/github/work-item-search-test-harness.ts (4) - one consumer lives in tests/e2e, where the relative mock ids resolve differently, so moving the calls into the specs would silently stop mocking there. - src/renderer/src/components/automations/automations-page-test-harness.tsx (14) - the mount rig for 10 AutomationsPage specs. - src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts (1) - stubs refreshWebRuntimeSessionTabsSnapshot, imported directly by several renderer runtime modules; 18 specs share it. - src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts (7) - stubReactSyncEffect/stubAuxiliaryModules, shared by 11 specs. - src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts (11) - stubs and hook invocation are one unit; 4 specs share it. - src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts (13) - its only spec is at 799 of an 800 max-lines budget. - src/renderer/src/hooks/ipc-events-test-harness.ts (10) - shared by 8 specs. No violation was converted to real dependency injection, and no max-lines disable was added. Verified: the audit command exits 0 with no output (and reports errors on a planted probe, so the rule is live); node config/scripts/run-typecheck-projects-in-parallel.mjs exits 0; 354 spec files / 2506 tests covering every importer of every touched file pass. No mobile/ file was touched. The changed-code quality gate's root Oxlint scan runs without --config so it never loads the anti-slop JS plugin, which made all 10 of those file-level suppressions read as "Unused oxlint-disable directive". check-changed-code-quality.mjs now exempts directives naming an anti-slop rule from that unused-directive warning, the same carve-out isCastingDirectiveUnusedWarning already makes for the casting suppressions the casting config enforces. Such a directive can never suppress a root-config rule, so nothing the root scan would otherwise report is hidden; audit:anti-slop remains the scan that enforces the rule.	2026-09-15 00:41:17 -07:00
Neil	49e5fa597a	refactor(lint): enable anti-slop/no-reflect-apply (#20782) ... `anti-slop/no-reflect-apply` rejects `Reflect.apply(fn, thisArg, argsArray)`. It defeats the call-signature checks TypeScript applies to an ordinary call: the args array is checked as an array, not positionally against the callee's parameters, so arity and type errors pass silently. Dynamic dispatch belongs behind a named interface, not behind a reflective call. Flipped the rule from "off" to "error" and cleared all 17 baseline violations across `src config tests mobile` (16 sites; one file had two). Fix pattern: `Reflect.apply(fn, recv, args)` becomes `fn.call(recv, ...args)`, or a direct method call when the implicit receiver is already the right object. The receiver is preserved at every site. Where the callee is a captured built-in whose overloads split on an argument's shape (`String.prototype.split`, `JSON.stringify`), a call-signature capture no longer compiles once the args are passed positionally. Those three sites capture the function through a method-shaped type (`{ split(separator: unknown, limit?: number): string[] }['split']`), which keeps the forwarding call checked rather than asserted. Behaviour notes: - `diff-section-layout.test.ts` drops a `limit === undefined ? [sep] : [sep, limit]` conditional. Equivalent: `String.prototype.split` maps an undefined limit to 2^32-1, and the `Symbol.split` path forwards undefined either way. - `workspace-space-compaction.test.ts` forwards `reduce`'s two arguments unchanged, so the `arguments.length >= 2` initial-value branch is unaffected. - `agent-session-history-byte-accounting.test.ts` is the one site where the receiver is not literally preserved (`JSON` -> undefined). `JSON.stringify` never reads `this` per spec, and restoring `.call(JSON, ...)` would reintroduce the overload failure under strictBindCallApply. No suppression comments added — the rule has zero `oxlint-disable` sites. `Reflect.apply` still appears at electron.vite.config.ts:159, inside a template literal of generated bootstrap source. That is string content, not lintable code.	2026-09-15 00:10:11 -07:00
Neil	18d0afc918	test(package): let the postinstall contract allow unrelated chained steps (#20787)	2026-09-14 23:26:48 -07:00
Neil	11180fa532	chore(lint): add anti-slop oxlint plugin (pinned, all rules off) (#20726) ... * chore(lint): add anti-slop oxlint plugin (all rules off) Vendors dmmulroy/anti-slop (MIT) plus no-call-only-assertions and no-pass-through-type-alias from maharshi365/deslop (MIT). Every rule starts "off"; each follow-up PR fixes one rule's violations and flips it to "error". * fix(lint): actually exclude the vendored plugin from the anti-slop audit oxlint does not honour ignorePatterns supplied via --config, so the config/oxlint-plugins/anti-slop/** entry never matched and the vendored rule source was being linted as first-party code (505 violations). Move the exclusion to the --ignore-pattern CLI flag in audit:anti-slop, which does work, and drop the entry that gave a false sense of coverage. Keeping vendored source unlinted matters because anti-slop is updated by three-way merge against the upstream snapshot; reformatting it locally would conflict on every update. * chore(lint): pin anti-slop instead of vendoring it; drop deslop Replaces the ~5k vendored lines with a git-pinned devDependency: oxlint-plugin-anti-slop: github:dmmulroy/anti-slop#c44ef22 anti-slop ships raw .ts with no build step, and Node refuses to type-strip anything under node_modules (ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING), so oxlint cannot load it from there -- which is why upstream says to vendor it. A postinstall step copies the pinned package's source to .anti-slop-plugin/ (gitignored), which Node will type-strip because it sits outside node_modules. Upgrading is now a SHA bump rather than a re-vendor and three-way merge. Verified byte-identical rule output to the vendored copy across all 16 rules that fire. Drops maharshi365/deslop and its two rules (no-call-only-assertions, no-pass-through-type-alias). It is not on npm either, so it would need a second git pin and copy step, and it is a 5-star single-maintainer repo that is itself a re-namespaced copy of anti-slop. One upstream is enough. * ci(lint): run audit:anti-slop in PR CI config/scripts/pr-workflow-lint-parity.test.mjs requires every step in `pnpm lint` to have a matching step in .github/workflows/pr.yml; adding audit:anti-slop to lint without the workflow step failed that ratchet. Also makes audit:anti-slop sync the plugin itself before linting. The generated .anti-slop-plugin/ directory is gitignored and otherwise only created by postinstall, so a cached install that skips postinstall would leave oxlint unable to load the plugin.	2026-09-14 21:42:37 -07:00
Neil	b61a2347b9	feat(design-system): gate renderer UI with @shadcn/lint (#20731) ... * feat(design-system): gate renderer UI with @shadcn/lint Wires shadcn-ui/lint's Oxlint plugin into the two places this repo already ratchets: the changed-lines PR gate for rules the renderer can't satisfy today, and `pnpm lint` for the one that is already at zero. - config/oxlint-design-system.json: no-restyle (layout allowed), no-raw-colors, require-static-classes -- scoped to src/renderer/**/*.tsx, run over added lines only. Measured at 10 findings across the last 60 commits (771 changed files), so it holds the line without a migration. - config/oxlint-dead-classes.json: no-unknown-classes repo-wide, with the renderer's plain-CSS hook namespaces allow-listed. Now at zero. - no-inline-styles and no-arbitrary-values stay off; STYLEGUIDE says why. Fixes the three live bugs the linter found: - `--editor-surface` never reached `@theme inline`, so `bg-editor-surface` generated no CSS -- 12 editor/artifact/notebook panes fell through to the page background instead of #1e1e1e in dark mode. - `scrollbar-none` is not a Tailwind utility and was declared nowhere, so the remote file browser breadcrumbs showed the scrollbar they meant to hide. Declared as a real `@utility`. - Notebook markdown cells used `markdown-preview-body`, which no stylesheet defines; the styled class is `markdown-body`. They rendered unstyled. * ci: run the dead-class gate in PR CI `pnpm lint` gained check:dead-classes, and pr-workflow-lint-parity requires every `pnpm lint` step to have a matching step in pr.yml. * fix(notebook): keep markdown theme selectors working	2026-09-14 17:52:21 -07:00
Neil	20794ee785	ci: keep the baseline build off the compatibility matrix lanes (#20733) ... The compatibility gate started the pinned 2.25.5 source build inside the same step that runs the three measured lanes, so `make -j$(nproc)` competed with two container lanes whose wall clock is container starts, not Git. A boundary case that costs ~1.5s stretched past Vitest's 30s timeout and failed the job. Build the binary in its own step before the matrix, and pull both images before any lane starts so a lazy pull cannot stall whichever test its sibling is timing.	2026-09-14 17:32:33 -07:00
Neil	fc4519cda4	fix(omp): preserve zsh startup with global aliases (#20621) ... Validated and independently reviewed OMP integration fix.	2026-09-14 13:56:22 -07:00
Neil	1ba9801574	fix(ci): stop hourly versions dropping below a tagged or already-shipped build (#20699) ... * fix(ci): stop hourly versions dropping below a tagged or already-shipped build Hourly/daily/adhoc based their X.Y.Z on GitHub releases, not git tags. When v1.4.202 was tagged and then its GitHub release vanished, the next hourlies shipped as 1.4.202-hourly — below both stable 1.4.202 and the 1.4.203-hourly builds already installed, so electron-updater stopped offering updates. Read main's v* tags and already-published channel tags instead. * docs(ci): record that 1.4.202's release was unpublished for a bug The leftover tag is what hourly must still honor; this was not a failed cut.	2026-09-14 13:33:04 -07:00