From e253d13a48461cae65c5115a7dbd339dd8430518 Mon Sep 17 00:00:00 2001 From: OrcaWin <293788423+OrcaWin@users.noreply.github.com> Date: Tue, 28 Jul 2026 07:25:29 -0700 Subject: [PATCH] fix(mobile): preserve hosted source control parity --- config/scripts/package-mobile-web-rnw.mjs | 2 +- config/scripts/package-mobile-web-rnw.test.ts | 1 + .../scripts/verify-mobile-web-rnw-build.mjs | 3 + ...hybrid-webview-implementation-checklist.md | 4 + ...-mobile-hybrid-webview-parity-inventory.md | 76 ++++++----- ...bile-hybrid-webview-single-pr-migration.md | 37 ++++-- ...27-mobile-hybrid-webview-remaining-work.md | 25 ++-- .../hosted-ios-emulator-accessibility.mjs | 48 +++++++ .../hosted-ios-emulator-long-press.mjs | 27 ++++ ...sted-ios-source-control-review-journey.mjs | 69 +++++++++- ...osted-ios-source-control-review-parity.mjs | 125 ++++++++++++++++++ .../run-hosted-webview-simulator-e2e.mjs | 19 +++ .../src/components/MobileSyntaxSegments.tsx | 10 +- .../hosted-ios-emulator-accessibility.test.ts | 51 +++++++ .../hosted-ios-emulator-long-press.test.ts | 45 +++++++ ...-ios-source-control-review-journey.test.ts | 53 +++++++- ...d-ios-source-control-review-parity.test.ts | 121 +++++++++++++++++ .../mobile-web-source-control-compare-page.ts | 44 ++++++ ...-source-control-history-operations.test.ts | 79 +++++++++++ ...e-web-source-control-history-operations.ts | 2 +- ...e-web-source-control-history-sanitizers.ts | 68 +++++++--- .../web-host-diff-review-client.test.ts | 93 +++++++++++++ .../web-host-provider-review-creation.ts | 71 +++++++++- .../web-host-provider-review-loader.ts | 81 ++++++++++++ .../web-host-provider-review-requests.test.ts | 83 ++++++++++++ .../web-host-provider-review-requests.ts | 55 ++------ .../web-host-source-control-client.ts | 8 +- .../web-host-source-control-reads.ts | 53 +++++++- ...ile-web-source-control-repository.test.tsx | 2 +- ...-web-source-control-request-client.test.ts | 8 +- ...obile-web-source-control-request-client.ts | 6 +- ...se-mobile-web-source-control-repository.ts | 2 +- .../source-control-history-contract.test.ts | 4 + .../source-control-history-contract.ts | 23 +++- 34 files changed, 1264 insertions(+), 134 deletions(-) create mode 100644 mobile/scripts/hosted-ios-emulator-long-press.mjs create mode 100644 mobile/scripts/hosted-ios-source-control-review-parity.mjs create mode 100644 mobile/src/mobile-web/hosted-ios-emulator-long-press.test.ts create mode 100644 mobile/src/mobile-web/hosted-ios-source-control-review-parity.test.ts create mode 100644 mobile/src/mobile-web/mobile-web-source-control-compare-page.ts create mode 100644 mobile/src/source-control/web-host-provider-review-loader.ts diff --git a/config/scripts/package-mobile-web-rnw.mjs b/config/scripts/package-mobile-web-rnw.mjs index 73ec6c9f509..db5d2d41821 100644 --- a/config/scripts/package-mobile-web-rnw.mjs +++ b/config/scripts/package-mobile-web-rnw.mjs @@ -174,7 +174,7 @@ function mobileWebDocument({ scriptPath, stylePath }) { - + Orca diff --git a/config/scripts/package-mobile-web-rnw.test.ts b/config/scripts/package-mobile-web-rnw.test.ts index 2f2f59e2b11..7acd9e1bd84 100644 --- a/config/scripts/package-mobile-web-rnw.test.ts +++ b/config/scripts/package-mobile-web-rnw.test.ts @@ -79,6 +79,7 @@ describe('RNW mobile web packager', () => { expect(document).not.toContain("script-src 'self' 'unsafe-inline'") expect(document).not.toContain(' output }) diff --git a/config/scripts/verify-mobile-web-rnw-build.mjs b/config/scripts/verify-mobile-web-rnw-build.mjs index ca14f0ed49a..3cb8e7d5d3d 100644 --- a/config/scripts/verify-mobile-web-rnw-build.mjs +++ b/config/scripts/verify-mobile-web-rnw-build.mjs @@ -73,6 +73,9 @@ if (JSON.stringify(actualPaths) !== JSON.stringify(declaredPaths)) { } const html = await readFile(path.join(outputRoot, manifest.entrypoint), 'utf8') +if (!/]*\bviewport-fit=cover\b/i.test(html)) { + throw new Error('RNW document must expose native safe-area insets') +} const requiredCsp = [ "default-src 'none'", `script-src 'self' ${MOBILE_RICH_MARKDOWN_EDITOR_SCRIPT_CSP_HASH}`, diff --git a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md index 9570b3cfddc..df9011c2cbf 100644 --- a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md +++ b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-implementation-checklist.md @@ -2465,4 +2465,8 @@ copy. | 2026-07-28 | Complete | Post-Accounts validation passes 554 mobile files / 3,299 tests with 2 expected skips, mobile and RNW typechecks, mobile lint, full mobile formatting, max-lines, and diff hygiene. The RNW package remains unchanged because this slice adds only the native-versus-hosted driver and evidence. | | 2026-07-28 | Complete | The unchanged base workspace `HostScreen` passes iPhone 17 Pro native-versus-hosted parity at 0.879% changed pixels / 1.876 mean channel difference / 0.000395 vertical landmark delta against 3% / 4 / 0.005 budgets. The complete cached-app journey captures Workspace before Accounts, then passes every existing downstream parity, recovery, review, and isolation checkpoint. | | 2026-07-28 | Complete | Post-Workspace validation passes 555 mobile files / 3,301 tests with 2 expected skips, mobile and RNW typechecks, mobile lint, full mobile formatting, max-lines, and diff hygiene. The RNW package remains unchanged because this slice adds only deterministic parity infrastructure and evidence. | +| 2026-07-28 | Complete | Hosted `git.branchCompare` now returns revision-consistent pages of at most 128 entries with the existing byte budget and a 4,000-entry aggregate ceiling. The page adapter assembles every page and rejects changed revisions, so native and hosted Review both show all 1,294 files, the same first file, and the same diff. | +| 2026-07-28 | Complete | The unchanged Source Control and Review screens pass the cached iPhone 17 Pro journey against the 1,294-file comparison. Source Control measures 0.736% changed pixels / 0.910 mean channel difference; Review measures 2.134% / 1.947 against 3% / 4 budgets. The packaged document exposes native safe-area insets and nested syntax text retains native font behavior. | +| 2026-07-28 | Complete | Final slice validation passes 557 mobile files / 3,312 tests with 2 expected skips and 4 directly affected root files / 17 tests. Mobile and RNW typechecks/lints, full mobile and changed-file formatting, max-lines, package verification, and diff hygiene pass. Package `190a35ea6c3e53ffa099afba9da1103acbbd8d1b165bf1c0e19391e6d026c2c1` contains 49 assets, 9,333,750 raw bytes, and 2,698,592 gzip bytes. | +| 2026-07-28 | Finding | Repository-wide formatting still reports 19 unrelated baseline files; every migration-owned changed file passes formatting. | | 2026-07-28 | Next | Complete the remaining parity inventory and cutover cleanup, then execute the physical-device, topology, security, performance, packaged-release, and App Store gates. | diff --git a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md index 7deb431be61..e60359f308f 100644 --- a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md +++ b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-parity-inventory.md @@ -42,38 +42,38 @@ not satisfy parity. `QuickCommandsTabButton.tsx` is colocated under the Expo Router session folder but is a component, not a route. It migrates with the session UI. -| Current route | Current responsibility | Target owner | Migration decision | -| ------------------------------------------------------- | ---------------------------------------------------------------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `mobile/app/_layout.tsx` | Root providers, deep links, notification routing, pairing recovery, route registry | Native shell | Keep; replace prototype registration with production hybrid route | -| `mobile/app/index.tsx` | Host list plus cross-host worktree/task/account summaries | Native shell | Keep host selection and health; move host workspace summaries/actions behind host entry | -| `mobile/app/mobile-onboarding.tsx` | Notification and session-view onboarding | Native shell | Keep; update claims and default destination for hybrid workspace | -| `mobile/app/pair-scan.tsx` | QR/manual pairing and pre-profile connection | Native shell | Keep | -| `mobile/app/pair.tsx` | Pairing deep-link redirect | Native shell | Keep | -| `mobile/app/pair-confirm.tsx` | Pair confirmation, credential installation, connection feedback | Native shell | Keep | -| `mobile/app/settings.tsx` | Native app settings navigation and credential cleanup retry | Native shell | Keep; remove Experimental prototype entry at cutover | -| `mobile/app/terminal-settings.tsx` | Terminal input, shortcut, font, and recovery preferences | Native shell | Keep native/device preferences; expose typed values to web terminal | -| `mobile/app/native-chat-settings.tsx` | Default session view preference | Native shell | Keep preference; web session consumes it through shell state | -| `mobile/app/browser-settings.tsx` | Browser interaction preferences | Native shell | Keep device/input preferences; web browser surface consumes them | -| `mobile/app/voice-settings.tsx` | Dictation model setup and host voice capabilities | Native shell | Keep permission/model lifecycle; web invokes typed audio/dictation capabilities | -| `mobile/app/notifications.tsx` | Notification permission and preference UI | Native shell | Keep | -| `mobile/app/notification-opt-in.tsx` | Legacy notification route redirect | Native shell | Keep until legacy deep-link support can be retired independently | -| `mobile/app/troubleshoot.tsx` | Reachability, diagnostics, and recovery actions | Native shell | Keep; add package/cache/bridge recovery state | -| `mobile/app/connection-log.tsx` | Connection diagnostics and support report | Native shell | Keep; add privacy-safe hybrid diagnostics | -| `mobile/app/about.tsx` | Native application identity/version | Native shell | Keep; add shell and active web build versions where useful | -| `mobile/app/h/_layout.tsx` | Host protocol gate and host route stack | Native shell | Keep as host security/recovery boundary; mount one production hybrid workspace route | -| `mobile/app/h/[hostId]/edit.tsx` | Paired host display name, endpoint, reconnect | Native shell | Keep because it changes paired connectivity rather than workspace content | -| `mobile/app/h/[hostId]/index.tsx` | Worktree list, creation, actions, host workspace entry | Mobile web app | Complete on iOS Simulator: native and hosted mount the same `HostScreen` and pass strict screenshot parity | -| `mobile/app/h/[hostId]/accounts.tsx` | Host agent-account usage and selection | Mobile web app | Complete on iOS Simulator: the same screen uses typed native/web host-account adapters and passes strict screenshot parity | -| `mobile/app/h/[hostId]/tasks.tsx` | Host task providers, task details, mutations, workspace creation | Mobile web app | Complete on iOS Simulator: same route/presentation with strict native/web operations | -| `mobile/app/h/[hostId]/session/[worktreeId].tsx` | Sessions, tabs, terminal, browser, native chat, files, attachments, dictation | Mobile web app | Reuse current session presentation; native-chat/agent-state slice is adapter-complete and awaiting live parity evidence | -| `mobile/app/h/[hostId]/agent-history/[worktreeId].tsx` | Agent session history and resume | Mobile web app | Complete on iOS and Android emulators: same panel/list presentation, bounded opaque snapshot/preview/resume operations, scopes, search, and trusted-resume mediation | -| `mobile/app/h/[hostId]/files/[worktreeId].tsx` | File explorer | Mobile web app | Complete at adapter level: the same `MobileFileExplorerPanel` is mounted by a thin hosted route with bounded opaque directory reads and native-shell reconnect | -| `mobile/app/h/[hostId]/files/preview/[worktreeId].tsx` | File, Markdown, image, editable, HTML, and Mermaid previews | Mobile web app | Reuse current preview UI; HTML is sanitized in a hash-bound inert frame, while native/hosted Mermaid use the locally bundled hash-authorized engine and unchanged fallback presentation | -| `mobile/app/h/[hostId]/source-control/[worktreeId].tsx` | Source-control hub | Mobile web app | Complete on iOS and Android emulators: same `MobileSourceControlPanel`, including Session-origin changed-file handoff | -| `mobile/app/h/[hostId]/review/[worktreeId].tsx` | Diff review and comments | Mobile web app | Complete on iOS and Android emulators: same review presentation and virtualization; standalone controls verified independently | -| `mobile/app/h/[hostId]/history/[worktreeId].tsx` | Legacy source-control history redirect | Mobile web app | Complete: provider-neutral compatibility redirect preserves the history segment during rollout | -| `mobile/app/h/[hostId]/pr/[worktreeId].tsx` | Legacy pull-request redirect | Mobile web app | Complete: provider-neutral compatibility redirect preserves the pull-request segment during rollout | -| `mobile/app/hybrid-prototype.tsx` | Experimental single-document Option B prototype | Remove | Replace with production host route after all gates pass | +| Current route | Current responsibility | Target owner | Migration decision | +| ------------------------------------------------------- | ---------------------------------------------------------------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `mobile/app/_layout.tsx` | Root providers, deep links, notification routing, pairing recovery, route registry | Native shell | Keep; replace prototype registration with production hybrid route | +| `mobile/app/index.tsx` | Host list plus cross-host worktree/task/account summaries | Native shell | Keep host selection and health; move host workspace summaries/actions behind host entry | +| `mobile/app/mobile-onboarding.tsx` | Notification and session-view onboarding | Native shell | Keep; update claims and default destination for hybrid workspace | +| `mobile/app/pair-scan.tsx` | QR/manual pairing and pre-profile connection | Native shell | Keep | +| `mobile/app/pair.tsx` | Pairing deep-link redirect | Native shell | Keep | +| `mobile/app/pair-confirm.tsx` | Pair confirmation, credential installation, connection feedback | Native shell | Keep | +| `mobile/app/settings.tsx` | Native app settings navigation and credential cleanup retry | Native shell | Keep; remove Experimental prototype entry at cutover | +| `mobile/app/terminal-settings.tsx` | Terminal input, shortcut, font, and recovery preferences | Native shell | Keep native/device preferences; expose typed values to web terminal | +| `mobile/app/native-chat-settings.tsx` | Default session view preference | Native shell | Keep preference; web session consumes it through shell state | +| `mobile/app/browser-settings.tsx` | Browser interaction preferences | Native shell | Keep device/input preferences; web browser surface consumes them | +| `mobile/app/voice-settings.tsx` | Dictation model setup and host voice capabilities | Native shell | Keep permission/model lifecycle; web invokes typed audio/dictation capabilities | +| `mobile/app/notifications.tsx` | Notification permission and preference UI | Native shell | Keep | +| `mobile/app/notification-opt-in.tsx` | Legacy notification route redirect | Native shell | Keep until legacy deep-link support can be retired independently | +| `mobile/app/troubleshoot.tsx` | Reachability, diagnostics, and recovery actions | Native shell | Keep; add package/cache/bridge recovery state | +| `mobile/app/connection-log.tsx` | Connection diagnostics and support report | Native shell | Keep; add privacy-safe hybrid diagnostics | +| `mobile/app/about.tsx` | Native application identity/version | Native shell | Keep; add shell and active web build versions where useful | +| `mobile/app/h/_layout.tsx` | Host protocol gate and host route stack | Native shell | Keep as host security/recovery boundary; mount one production hybrid workspace route | +| `mobile/app/h/[hostId]/edit.tsx` | Paired host display name, endpoint, reconnect | Native shell | Keep because it changes paired connectivity rather than workspace content | +| `mobile/app/h/[hostId]/index.tsx` | Worktree list, creation, actions, host workspace entry | Mobile web app | Complete on iOS Simulator: native and hosted mount the same `HostScreen` and pass strict screenshot parity | +| `mobile/app/h/[hostId]/accounts.tsx` | Host agent-account usage and selection | Mobile web app | Complete on iOS Simulator: the same screen uses typed native/web host-account adapters and passes strict screenshot parity | +| `mobile/app/h/[hostId]/tasks.tsx` | Host task providers, task details, mutations, workspace creation | Mobile web app | Complete on iOS Simulator: same route/presentation with strict native/web operations | +| `mobile/app/h/[hostId]/session/[worktreeId].tsx` | Sessions, tabs, terminal, browser, native chat, files, attachments, dictation | Mobile web app | Complete at route level on iOS Simulator: the same Session presentation passes strict screenshot parity, terminal interaction, native-chat recovery, and downstream route handoff | +| `mobile/app/h/[hostId]/agent-history/[worktreeId].tsx` | Agent session history and resume | Mobile web app | Complete on iOS and Android emulators: same panel/list presentation, bounded opaque snapshot/preview/resume operations, scopes, search, and trusted-resume mediation | +| `mobile/app/h/[hostId]/files/[worktreeId].tsx` | File explorer | Mobile web app | Complete on iOS Simulator: the same `MobileFileExplorerPanel` passes strict screenshot parity with bounded opaque directory reads and native-shell reconnect | +| `mobile/app/h/[hostId]/files/preview/[worktreeId].tsx` | File, Markdown, image, editable, HTML, and Mermaid previews | Mobile web app | Complete on iOS Simulator for real file Preview parity; HTML remains sanitized in a hash-bound inert frame and native/hosted Mermaid use the bundled hash-authorized engine | +| `mobile/app/h/[hostId]/source-control/[worktreeId].tsx` | Source-control hub | Mobile web app | Complete on iOS and Android emulators: same `MobileSourceControlPanel`, including Session-origin changed-file handoff | +| `mobile/app/h/[hostId]/review/[worktreeId].tsx` | Diff review and comments | Mobile web app | Complete on iOS and Android emulators: same review presentation and virtualization; standalone controls verified independently | +| `mobile/app/h/[hostId]/history/[worktreeId].tsx` | Legacy source-control history redirect | Mobile web app | Complete: provider-neutral compatibility redirect preserves the history segment during rollout | +| `mobile/app/h/[hostId]/pr/[worktreeId].tsx` | Legacy pull-request redirect | Mobile web app | Complete: provider-neutral compatibility redirect preserves the pull-request segment during rollout | +| `mobile/app/hybrid-prototype.tsx` | Experimental single-document Option B prototype | Remove | Replace with production host route after all gates pass | ## RPC and Subscription Inventory @@ -508,8 +508,18 @@ channel difference / 0.000395 vertical landmark delta, within the 3% / 4 / 0.005 budgets. This proves the shared `HostScreen` itself before the same fixture navigates into Accounts and the downstream route matrix. +The Source Control/Review fixture now runs against the full 1,294-file branch +comparison rather than a byte-truncated prefix. Desktop responses are bounded +to 128 revision-consistent entries per page and 4,000 entries in aggregate; +the hosted adapter assembles the pages and rejects a revision change. Native +and hosted show the same `0/1294 reviewed` state, first file, and diff without +forking the shared presentation. Source Control measures 0.736% changed pixels +/ 0.910 mean channel difference and Review measures 2.134% / 1.947, within the +3% / 4 budgets. `viewport-fit=cover` supplies native safe-area insets, and RNW +nested syntax text follows the native effective font. + The interrupted-transcript versus hook-status mismatch and a real structured -prompt response pass Host 37 Simulator replay. Current package `4b7df7d4…` +prompt response pass Host 37 Simulator replay. Current package `190a35ea…` also carries the network-denied local Mermaid engine and its WebKit-compatible token-bound parent/frame handoff. Classic SSH transcript authority and reconnect now pass a real Docker provider journey diff --git a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md index f47232e8b97..7d6e6f3edf6 100644 --- a/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md +++ b/docs/reference/plans/2026-07-22-mobile-hybrid-webview-single-pr-migration.md @@ -1020,20 +1020,31 @@ stale terminal input with Ctrl+U. The standalone React Native Web packager reads the protocol version from a browser-pure shared module, and the bridge protocol is version 2. +The final Source Control scale checkpoint preserves the same native Review +presentation for a real 1,294-file comparison. Desktop `git.branchCompare` +responses are revision-consistent pages of at most 128 entries, with the +existing response-byte budget and a 4,000-entry aggregate ceiling. The hosted +adapter assembles all pages and rejects a revision change instead of mixing +snapshots. Native and hosted both render `0/1294 reviewed`, the same first file, +and the same diff. The packaged RNW document opts into native safe-area insets, +and nested syntax text keeps the effective native font behavior. On iPhone 17 +Pro Simulator, Source Control passes at 0.736% changed pixels / 0.910 mean +channel difference and Review at 2.134% / 1.947, within the 3% / 4 budgets. + Current validation passes mobile and mobile-web typechecks and lints, mobile -formatting, max-lines and diff hygiene, 517 mobile files / 3,150 tests with 2 -expected skips, Android native unit/Debug/Release compilation, and the -independent RNW verifier. The most recent full root run passes 3,548 files / -37,404 tests with 60 skips. Focused package/session/broker faults pass 43 mobile -tests; bridge/channel faults pass 52 root tests; the Swift cache executable and -all 16 Android native store tests pass. Production package -`bb86b378f0aa0285b07793558d27647411bf61185b12af8b10682df23969c97e` -contains 49 assets and verifies at 9,179,679 raw bytes / 2,663,276 gzip bytes. -Full root lint reaches only an unrelated baseline localization-coverage failure -for six unchanged `Ghostty` search keywords; migration-owned lint is green. -This evidence does not close the complete Android matrix, physical-device, -independent-security, performance, signed release-package, physical/final -rollback, cloud Relay, or App Review gates. +formatting, max-lines and diff hygiene, 557 mobile files / 3,312 tests with 2 +expected skips, and 17 directly affected root tests. The independently verified +production package +`190a35ea6c3e53ffa099afba9da1103acbbd8d1b165bf1c0e19391e6d026c2c1` +contains 49 assets and verifies at 9,333,750 raw bytes / 2,698,592 gzip bytes. +The complete cached-app iOS journey passes Workspace, Accounts, Tasks, Session, +Files/Preview, Agent History portrait/landscape, Desktop restart and E2EE +recovery, native-touch resume, Source Control, a third Session diff tab, +standalone Review, and both private-origin isolation probes. Repository-wide +formatting still reports 19 unrelated baseline files; all migration-owned files +are formatted. This evidence does not close physical-device, +independent-security, sustained-performance, signed release-package, +physical/final rollback, production cloud Relay, or App Review gates. The earlier `77708ed1…` checkpoint also proved that a host-created terminal appeared through the subscription and disappeared after close without a refresh, diff --git a/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md b/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md index 6c14df79813..81914927dd1 100644 --- a/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md +++ b/docs/reference/plans/2026-07-27-mobile-hybrid-webview-remaining-work.md @@ -79,20 +79,29 @@ hosted mount the unchanged `HostScreen` and pass at 0.879% changed pixels, 3% / 4 / 0.005 budgets. The complete journey captures this screen before Accounts and the rest of the route matrix. +The unchanged Source Control and Review screens now also have scale-correct +parity evidence against the real 1,294-file branch comparison. The Desktop +serves revision-consistent pages of at most 128 entries with a 4,000-entry +aggregate ceiling, and the hosted adapter assembles those pages without +changing the presentation. Native and hosted both show `0/1294 reviewed`, the +same first file, and the same diff. Source Control passes at 0.736% changed +pixels and 0.910 mean channel difference; Review passes at 2.134% and 1.947, +within the 3% / 4 budgets. The packaged document opts into native safe-area +insets, and nested syntax text retains the native effective font behavior. + The migration is rebased onto `origin/main` at `0404f27b3`. Current post-rebase validation passes 552 mobile files / 3,291 tests with 2 expected skips and 3,770 root files / 39,212 tests with 62 expected skips. All project typechecks, root/mobile/mobile-web lint, reliability gates, changed-file and full-mobile formatting, localization, and the max-lines ratchet pass. The independently verified React Native Web package is -`9e5e807523e8b917fef68f221cc1fd2e1a16dbe07d7077e717238eed17003b52`: -49 assets, 9,330,604 raw bytes, and 2,697,919 gzip bytes. The current mobile -suite passes 555 files / 3,301 tests with 2 expected skips; all project -typechecks and repository-wide quality gates pass. A fresh root-suite attempt -hit an unrelated 30-second timeout in -`project-view-wrapper-source-context-boundary.test.ts` and was interrupted -under concurrent test load; that exact file passes alone, so the prior complete -root-suite checkpoint above remains authoritative. +`190a35ea6c3e53ffa099afba9da1103acbbd8d1b165bf1c0e19391e6d026c2c1`: +49 assets, 9,333,750 raw bytes, and 2,698,592 gzip bytes. The current mobile +suite passes 557 files / 3,312 tests with 2 expected skips. The four directly +affected root suites pass 17 tests; mobile and mobile-web typechecks and lints, +full mobile and changed-file formatting, max-lines, package verification, and +diff hygiene pass. The repository-wide formatter still reports 19 unrelated +baseline files, so changed-file formatting is the migration-owned gate. ## 1. Finish Hosted Feature Parity diff --git a/mobile/scripts/hosted-ios-emulator-accessibility.mjs b/mobile/scripts/hosted-ios-emulator-accessibility.mjs index 2d14e9863a3..65a100687de 100644 --- a/mobile/scripts/hosted-ios-emulator-accessibility.mjs +++ b/mobile/scripts/hosted-ios-emulator-accessibility.mjs @@ -60,6 +60,22 @@ export async function tapHostedIosAccessibilityControlByLabelPrefix( return point } +export async function tapHostedIosAccessibilityControlStartingWith( + args, + labelPrefix, + timeoutMs, + runCommand = runHostedIosEmulatorCommand +) { + const point = await waitForHostedIosAccessibilityControlStartingWith( + args, + labelPrefix, + timeoutMs, + runCommand + ) + await runCommand(args, ['tap', String(point.x), String(point.y)]) + return point +} + export async function waitForHostedIosAccessibilityControlByLabelPrefix( args, labelPrefix, @@ -76,6 +92,38 @@ export async function waitForHostedIosAccessibilityControlByLabelPrefix( ) } +export async function waitForHostedIosAccessibilityControlStartingWith( + args, + labelPrefix, + timeoutMs, + runCommand = runHostedIosEmulatorCommand +) { + return waitForHostedIosAccessibilityControlOccurrence( + args, + labelPrefix, + 0, + timeoutMs, + runCommand, + (node) => node.label?.startsWith(labelPrefix) || node.value?.startsWith(labelPrefix) + ) +} + +export async function waitForHostedIosAccessibilityControlEndingWith( + args, + labelSuffix, + timeoutMs, + runCommand = runHostedIosEmulatorCommand +) { + return waitForHostedIosAccessibilityControlOccurrence( + args, + labelSuffix, + 0, + timeoutMs, + runCommand, + (node) => node.label?.endsWith(labelSuffix) || node.value?.endsWith(labelSuffix) + ) +} + export async function waitForHostedIosAccessibilityControl( args, label, diff --git a/mobile/scripts/hosted-ios-emulator-long-press.mjs b/mobile/scripts/hosted-ios-emulator-long-press.mjs new file mode 100644 index 00000000000..610691afaf4 --- /dev/null +++ b/mobile/scripts/hosted-ios-emulator-long-press.mjs @@ -0,0 +1,27 @@ +import { + runHostedIosEmulatorCommand, + waitForHostedIosAccessibilityControlByLabelPrefix +} from './hosted-ios-emulator-accessibility.mjs' + +const LONG_PRESS_MOVE_FRAMES = 30 + +export async function longPressHostedIosAccessibilityControlByLabelPrefix( + args, + labelPrefix, + timeoutMs, + runCommand = runHostedIosEmulatorCommand +) { + const point = await waitForHostedIosAccessibilityControlByLabelPrefix( + args, + labelPrefix, + timeoutMs, + runCommand + ) + const holdFrames = Array.from({ length: LONG_PRESS_MOVE_FRAMES }, () => ({ + type: 'move', + ...point + })) + const gesture = [{ type: 'begin', ...point }, ...holdFrames, { type: 'end', ...point }] + await runCommand(args, ['gesture', JSON.stringify(gesture)]) + return point +} diff --git a/mobile/scripts/hosted-ios-source-control-review-journey.mjs b/mobile/scripts/hosted-ios-source-control-review-journey.mjs index 8330e9d9b13..f18533f1571 100644 --- a/mobile/scripts/hosted-ios-source-control-review-journey.mjs +++ b/mobile/scripts/hosted-ios-source-control-review-journey.mjs @@ -7,11 +7,18 @@ import { tapHostedIosAccessibilityControl, tapHostedIosPoint } from './hosted-ios-emulator-accessibility.mjs' +import { + captureHostedSourceControlReviewScreen, + sourceControlReviewParityEvidence +} from './hosted-ios-source-control-review-parity.mjs' import { navigateHostedWebViewRoute } from './hosted-webview-route-navigation.mjs' export async function verifyHostedSourceControlReviewJourney({ + deviceUdid, discoveryUrl, emulator, + nativeBaselines, + runtimeDirectory, sessionDocument, timeoutMs, expectedSessionDiffText = '2 tabs', @@ -26,7 +33,7 @@ export async function verifyHostedSourceControlReviewJourney({ tapPoint }) ) - const sourceState = await journeyStep('read populated Source Control state', () => + let sourceState = await journeyStep('read populated Source Control state', () => waitForChangedFileState(sourceControl, timeoutMs) ) for (const label of ['Changes', 'Pull Request', 'Commits', 'Refresh source control']) { @@ -41,6 +48,24 @@ export async function verifyHostedSourceControlReviewJourney({ if (!changedFileLabel) { throw new Error('Source Control has no changed file available for Review.') } + if (nativeBaselines) { + sourceState = await journeyStep('wait for stable Source Control parity state', () => + waitForSourceControlParityState(sourceControl, timeoutMs, sourceState) + ) + } + const hostedSourceControl = nativeBaselines + ? await journeyStep('capture Source Control parity', () => + captureHostedSourceControlReviewScreen({ + deviceUdid, + document: sourceControl, + nativeBaseline: nativeBaselines.sourceControl, + runtimeDirectory, + screenshotName: 'hosted-source-control-portrait.png', + title: 'Source Control', + timeoutMs + }) + ) + : null const sessionDiff = await journeyStep('wait for Session diff route', () => openSessionDiffRoute({ discoveryUrl, @@ -69,13 +94,37 @@ export async function verifyHostedSourceControlReviewJourney({ throw new Error(`Review is missing ${label}.`) } } + const hostedReview = nativeBaselines + ? await journeyStep('capture Review parity', () => + captureHostedSourceControlReviewScreen({ + deviceUdid, + document: review, + nativeBaseline: nativeBaselines.review, + runtimeDirectory, + screenshotName: 'hosted-review-portrait.png', + title: 'Changes', + timeoutMs + }) + ) + : null return { sourceControlRoute: sourceState.href, sourceControlSegments: ['Changes', 'Pull Request', 'Commits'], sessionDiffRoute: sessionDiff.href, reviewRoute: reviewState.href, - reviewControls: ['Back', 'Open review actions'] + reviewControls: ['Back', 'Open review actions'], + ...(hostedSourceControl && hostedReview + ? { + parityFixture: { + sourceControl: sourceControlReviewParityEvidence( + nativeBaselines.sourceControl, + hostedSourceControl + ), + review: sourceControlReviewParityEvidence(nativeBaselines.review, hostedReview) + } + } + : {}) } } @@ -169,6 +218,22 @@ async function waitForChangedFileState(document, timeoutMs) { return state ?? readHostedWebViewState(document) } +async function waitForSourceControlParityState(document, timeoutMs, initialState) { + const deadline = Date.now() + timeoutMs + let state = initialState + while (Date.now() < deadline) { + if ( + state.bodyText.includes('Create pull request') && + /\b\d+ on branch\b/.test(state.bodyText) + ) { + return state + } + await delay(250) + state = await readHostedWebViewState(document) + } + throw new Error('Source Control did not settle its branch and pull-request state.') +} + function standaloneReviewRoute(sourceControlHref) { const url = new URL(sourceControlHref) const pathname = url.pathname.replace('/source-control/', '/review/') diff --git a/mobile/scripts/hosted-ios-source-control-review-parity.mjs b/mobile/scripts/hosted-ios-source-control-review-parity.mjs new file mode 100644 index 00000000000..c7889ed7e48 --- /dev/null +++ b/mobile/scripts/hosted-ios-source-control-review-parity.mjs @@ -0,0 +1,125 @@ +import { execFile } from 'node:child_process' +import path from 'node:path' +import { promisify } from 'node:util' +import { dismissEmulatorDeveloperMenuIfPresent } from './emulator-developer-menu-dismissal.mjs' +import { + tapHostedIosAccessibilityControl, + tapHostedIosAccessibilityControlStartingWith, + waitForHostedIosAccessibilityControl, + waitForHostedIosAccessibilityControlByLabelPrefix, + waitForHostedIosAccessibilityControlEndingWith, + waitForHostedIosAccessibilityControlStartingWith +} from './hosted-ios-emulator-accessibility.mjs' +import { longPressHostedIosAccessibilityControlByLabelPrefix } from './hosted-ios-emulator-long-press.mjs' +import { assertHostedIosScreenshotParity } from './hosted-ios-screenshot-parity.mjs' +import { readHostedWebViewTextPoint } from './hosted-webview-cdp-session.mjs' + +const execFileAsync = promisify(execFile) +const CHANGED_FILE_LABEL_PREFIX = 'Open changed file ' + +export async function captureNativeSourceControlReviewBaselines({ + deviceUdid, + emulator, + expectedWorkspace, + runtimeDirectory, + timeoutMs +}) { + await dismissEmulatorDeveloperMenuIfPresent(emulator) + await longPressHostedIosAccessibilityControlByLabelPrefix(emulator, expectedWorkspace, timeoutMs) + await tapHostedIosAccessibilityControl(emulator, 'Source Control', timeoutMs) + await waitForHostedIosAccessibilityControlStartingWith( + emulator, + CHANGED_FILE_LABEL_PREFIX, + timeoutMs + ) + await waitForHostedIosAccessibilityControl(emulator, 'Create pull request', timeoutMs) + await waitForHostedIosAccessibilityControlEndingWith(emulator, ' on branch', timeoutMs) + const sourceControl = await captureNativeRoute({ + deviceUdid, + emulator, + runtimeDirectory, + screenshotName: 'native-source-control-portrait.png', + title: 'Source Control', + timeoutMs + }) + await tapHostedIosAccessibilityControlStartingWith(emulator, CHANGED_FILE_LABEL_PREFIX, timeoutMs) + await waitForHostedIosAccessibilityControl(emulator, 'Open review actions', timeoutMs) + await waitForHostedIosAccessibilityControlEndingWith(emulator, ' reviewed', timeoutMs) + const review = await captureNativeRoute({ + deviceUdid, + emulator, + runtimeDirectory, + screenshotName: 'native-review-portrait.png', + title: 'Changes', + timeoutMs + }) + await tapHostedIosAccessibilityControl(emulator, 'Back', timeoutMs) + await waitForHostedIosAccessibilityControl(emulator, 'Source Control', timeoutMs) + await tapHostedIosAccessibilityControl(emulator, 'Back to session', timeoutMs) + await waitForHostedIosAccessibilityControlByLabelPrefix(emulator, expectedWorkspace, timeoutMs) + return { review, sourceControl } +} + +export async function captureHostedSourceControlReviewScreen({ + deviceUdid, + document, + nativeBaseline, + runtimeDirectory, + screenshotName, + title, + timeoutMs +}) { + const screenTitlePoint = await readHostedWebViewTextPoint(document, title) + const screenshot = path.join(runtimeDirectory, screenshotName) + const deadline = Date.now() + timeoutMs + let lastError = new Error(`${title} did not reach screenshot parity`) + while (Date.now() < deadline) { + await delay(500) + await captureSimulatorScreenshot(deviceUdid, screenshot) + try { + const screenshotParity = await assertHostedIosScreenshotParity({ + hostedLandmark: screenTitlePoint, + hostedScreenshot: screenshot, + nativeLandmark: nativeBaseline.screenTitlePoint, + nativeScreenshot: nativeBaseline.screenshot + }) + return { screenTitlePoint, screenshot, screenshotParity } + } catch (error) { + lastError = error + } + } + throw lastError +} + +export function sourceControlReviewParityEvidence(nativeCapture, hostedCapture) { + return { + nativeScreenshot: path.basename(nativeCapture.screenshot), + hostedScreenshot: path.basename(hostedCapture.screenshot), + nativeScreenTitlePoint: nativeCapture.screenTitlePoint, + hostedScreenTitlePoint: hostedCapture.screenTitlePoint, + screenshotParity: hostedCapture.screenshotParity + } +} + +async function captureNativeRoute({ + deviceUdid, + emulator, + runtimeDirectory, + screenshotName, + title, + timeoutMs +}) { + const screenTitlePoint = await waitForHostedIosAccessibilityControl(emulator, title, timeoutMs) + await delay(500) + const screenshot = path.join(runtimeDirectory, screenshotName) + await captureSimulatorScreenshot(deviceUdid, screenshot) + return { screenTitlePoint, screenshot } +} + +async function captureSimulatorScreenshot(deviceUdid, outputPath) { + await execFileAsync('xcrun', ['simctl', 'io', deviceUdid, 'screenshot', outputPath]) +} + +function delay(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} diff --git a/mobile/scripts/run-hosted-webview-simulator-e2e.mjs b/mobile/scripts/run-hosted-webview-simulator-e2e.mjs index 00d4d4cefa5..3f4e99c24bc 100644 --- a/mobile/scripts/run-hosted-webview-simulator-e2e.mjs +++ b/mobile/scripts/run-hosted-webview-simulator-e2e.mjs @@ -37,6 +37,7 @@ import { verifyHostedAgentHistoryJourney } from './hosted-ios-agent-history-jour import { openHostedIosHybridRoute } from './hosted-ios-hybrid-route-handoff.mjs' import { verifyHostedNativeTerminalSettingsHandoff } from './hosted-ios-native-settings-handoff.mjs' import { verifyHostedSourceControlReviewJourney } from './hosted-ios-source-control-review-journey.mjs' +import { captureNativeSourceControlReviewBaselines } from './hosted-ios-source-control-review-parity.mjs' import { completeHostedIosNativeOnboarding } from './hosted-ios-native-onboarding.mjs' import { activateHostedWorkspaceRow } from './hosted-webview-workspace-activation.mjs' import { resolveHostedWebViewRuntimeDirectory } from './hosted-webview-runtime-directory.mjs' @@ -171,6 +172,21 @@ async function main() { timeoutMs: options.timeoutMs }) ) + const nativeSourceControlReview = + options.accountsOnly || + options.securityOnly || + options.filesPreviewOnly || + options.nativeSettingsOnly + ? null + : await evidenceStep('native Source Control and Review baselines', () => + captureNativeSourceControlReviewBaselines({ + deviceUdid, + emulator, + expectedWorkspace, + runtimeDirectory, + timeoutMs: options.timeoutMs + }) + ) const nativeAgentHistory = options.accountsOnly || options.securityOnly || @@ -326,9 +342,12 @@ async function main() { timeoutMs: options.timeoutMs }) return verifyHostedSourceControlReviewJourney({ + deviceUdid, discoveryUrl: `http://127.0.0.1:${inspectorPort}`, emulator, expectedSessionDiffText: options.sourceControlOnly ? '2 tabs' : '3 tabs', + nativeBaselines: nativeSourceControlReview, + runtimeDirectory, sessionDocument, timeoutMs: options.timeoutMs }) diff --git a/mobile/src/components/MobileSyntaxSegments.tsx b/mobile/src/components/MobileSyntaxSegments.tsx index 004ef48d945..446e3b09482 100644 --- a/mobile/src/components/MobileSyntaxSegments.tsx +++ b/mobile/src/components/MobileSyntaxSegments.tsx @@ -1,4 +1,4 @@ -import { StyleSheet, Text, type TextStyle } from 'react-native' +import { Platform, StyleSheet, Text, type TextStyle } from 'react-native' import type { MobileSyntaxSegment, MobileSyntaxTokenKind } from '../session/mobile-file-syntax' import { colors } from '../theme/mobile-theme' @@ -6,7 +6,10 @@ export function MobileSyntaxSegments({ segments }: { segments: MobileSyntaxSegme return ( <> {segments.map((segment, index) => ( - + {segment.text} ))} @@ -14,6 +17,9 @@ export function MobileSyntaxSegments({ segments }: { segments: MobileSyntaxSegme ) } +// Why: nested native Text resolves to the system face while RNW otherwise inherits the mono parent. +const webSyntaxTextStyle = Platform.OS === 'web' ? { fontFamily: 'System' } : undefined + const syntaxTokenStyles: Record = StyleSheet.create({ plain: { color: colors.textPrimary diff --git a/mobile/src/mobile-web/hosted-ios-emulator-accessibility.test.ts b/mobile/src/mobile-web/hosted-ios-emulator-accessibility.test.ts index 24d4621d20e..32d180ace33 100644 --- a/mobile/src/mobile-web/hosted-ios-emulator-accessibility.test.ts +++ b/mobile/src/mobile-web/hosted-ios-emulator-accessibility.test.ts @@ -4,8 +4,10 @@ import { tapHostedIosAccessibilityControl, tapHostedIosAccessibilityControlAtOccurrence, tapHostedIosAccessibilityControlByLabelPrefix, + tapHostedIosAccessibilityControlStartingWith, waitForHostedIosAccessibilityControl, waitForHostedIosAccessibilityControlByLabelPrefix, + waitForHostedIosAccessibilityControlEndingWith, waitForHostedIosAccessibilityLabelToDisappear } from '../../scripts/hosted-ios-emulator-accessibility.mjs' @@ -107,6 +109,35 @@ describe('hosted iOS emulator accessibility controls', () => { expect(runCommand).toHaveBeenLastCalledWith(emulator, ['tap', '0.5', '0.25']) }) + it('targets a dynamic control whose label starts with a stable action prefix', async () => { + const runCommand = vi + .fn() + .mockResolvedValueOnce({ + stderr: '', + stdout: JSON.stringify({ + ok: true, + result: [ + { + label: 'Open changed file mobile/app/index.tsx', + enabled: true, + frame: { x: 0.1, y: 0.3, width: 0.8, height: 0.1 } + } + ] + }) + }) + .mockResolvedValueOnce({ stderr: '', stdout: JSON.stringify({ ok: true }) }) + + await expect( + tapHostedIosAccessibilityControlStartingWith( + emulator, + 'Open changed file ', + 1_000, + runCommand + ) + ).resolves.toEqual({ x: 0.5, y: 0.35 }) + expect(runCommand).toHaveBeenLastCalledWith(emulator, ['tap', '0.5', '0.35']) + }) + it('reads a composite native row point without tapping it', async () => { const runCommand = vi.fn().mockResolvedValueOnce({ stderr: '', @@ -133,6 +164,26 @@ describe('hosted iOS emulator accessibility controls', () => { expect(runCommand).toHaveBeenCalledTimes(1) }) + it('waits for a dynamic count by its stable suffix', async () => { + const runCommand = vi.fn().mockResolvedValueOnce({ + stderr: '', + stdout: JSON.stringify({ + ok: true, + result: [ + { + label: '128 on branch', + enabled: true, + frame: { x: 0.6, y: 0.3, width: 0.3, height: 0.1 } + } + ] + }) + }) + + await expect( + waitForHostedIosAccessibilityControlEndingWith(emulator, ' on branch', 1_000, runCommand) + ).resolves.toEqual({ x: 0.75, y: 0.35 }) + }) + it('rejects an invalid accessibility response', async () => { await expect( waitForHostedIosAccessibilityControl(emulator, 'Resume agent session', 1_000, async () => ({ diff --git a/mobile/src/mobile-web/hosted-ios-emulator-long-press.test.ts b/mobile/src/mobile-web/hosted-ios-emulator-long-press.test.ts new file mode 100644 index 00000000000..357fb0874a1 --- /dev/null +++ b/mobile/src/mobile-web/hosted-ios-emulator-long-press.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it, vi } from 'vitest' +import { longPressHostedIosAccessibilityControlByLabelPrefix } from '../../scripts/hosted-ios-emulator-long-press.mjs' + +const emulator = { + deviceUdid: 'simulator-a', + orcaCli: '/repo/config/scripts/orca-dev.mjs', + userDataDir: '/tmp/orca-mobile/userData', + worktree: '/repo' +} + +describe('hosted iOS emulator long press', () => { + it('holds a composite workspace row beyond the React Native threshold', async () => { + const runCommand = vi + .fn() + .mockResolvedValueOnce({ + stderr: '', + stdout: JSON.stringify({ + ok: true, + result: [ + { + label: 'mobile-rearch, mobile-rearch, 1', + enabled: true, + frame: { x: 0, y: 0.2, width: 1, height: 0.1 } + } + ] + }) + }) + .mockResolvedValueOnce({ stderr: '', stdout: JSON.stringify({ ok: true }) }) + + await expect( + longPressHostedIosAccessibilityControlByLabelPrefix( + emulator, + 'mobile-rearch', + 1_000, + runCommand + ) + ).resolves.toEqual({ x: 0.5, y: 0.25 }) + + const gesture = JSON.parse(runCommand.mock.calls[1]?.[1][1] as string) + expect(runCommand.mock.calls[1]?.[1][0]).toBe('gesture') + expect(gesture).toHaveLength(32) + expect(gesture[0]).toEqual({ type: 'begin', x: 0.5, y: 0.25 }) + expect(gesture.at(-1)).toEqual({ type: 'end', x: 0.5, y: 0.25 }) + }) +}) diff --git a/mobile/src/mobile-web/hosted-ios-source-control-review-journey.test.ts b/mobile/src/mobile-web/hosted-ios-source-control-review-journey.test.ts index 8f65fc22c7f..83fde69f11c 100644 --- a/mobile/src/mobile-web/hosted-ios-source-control-review-journey.test.ts +++ b/mobile/src/mobile-web/hosted-ios-source-control-review-journey.test.ts @@ -1,7 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mocks = vi.hoisted(() => ({ + captureParityScreen: vi.fn(), navigateRoute: vi.fn(), + parityEvidence: vi.fn(), readControlPoint: vi.fn(), readState: vi.fn(), tapAccessibilityControl: vi.fn(), @@ -27,6 +29,11 @@ vi.mock('../../scripts/hosted-webview-route-navigation.mjs', () => ({ navigateHostedWebViewRoute: mocks.navigateRoute })) +vi.mock('../../scripts/hosted-ios-source-control-review-parity.mjs', () => ({ + captureHostedSourceControlReviewScreen: mocks.captureParityScreen, + sourceControlReviewParityEvidence: mocks.parityEvidence +})) + import { verifyHostedSourceControlReviewJourney } from '../../scripts/hosted-ios-source-control-review-journey.mjs' describe('hosted iOS Source Control and Review journey', () => { @@ -36,6 +43,12 @@ describe('hosted iOS Source Control and Review journey', () => { .mockResolvedValueOnce({ x: 0.4, y: 0.2 }) .mockResolvedValueOnce({ x: 0.5, y: 0.4 }) mocks.navigateRoute.mockResolvedValue(undefined) + mocks.captureParityScreen + .mockResolvedValueOnce({ screenshot: '/tmp/hosted-source-control.png' }) + .mockResolvedValueOnce({ screenshot: '/tmp/hosted-review.png' }) + mocks.parityEvidence + .mockReturnValueOnce({ screen: 'source-control' }) + .mockReturnValueOnce({ screen: 'review' }) mocks.tapAccessibilityControl.mockResolvedValue(undefined) mocks.tapPoint.mockResolvedValue(undefined) mocks.waitForDocument @@ -51,7 +64,7 @@ describe('hosted iOS Source Control and Review journey', () => { mocks.readState .mockResolvedValueOnce({ href: 'orca-mobile-web://build/h/host/source-control/workspace?name=repo&origin=session', - bodyText: 'Source Control Changes Pull Request Commits', + bodyText: 'Source Control Changes Pull Request Commits Create pull request 128 on branch', labels: ['Refresh source control', 'Open changed file mobile/app/index.tsx'] }) .mockResolvedValueOnce({ @@ -132,6 +145,44 @@ describe('hosted iOS Source Control and Review journey', () => { ) }) + it('captures strict Source Control and Review parity when native baselines are provided', async () => { + const result = await verifyHostedSourceControlReviewJourney({ + deviceUdid: 'simulator', + discoveryUrl: 'http://127.0.0.1:9222', + emulator: { deviceUdid: 'simulator' }, + nativeBaselines: { + sourceControl: { screenshot: '/tmp/native-source-control.png' }, + review: { screenshot: '/tmp/native-review.png' } + }, + runtimeDirectory: '/tmp/parity', + sessionDocument: { + href: 'orca-mobile-web://build/h/host/session/workspace' + }, + timeoutMs: 30_000 + }) + + expect(mocks.captureParityScreen).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + deviceUdid: 'simulator', + screenshotName: 'hosted-source-control-portrait.png', + title: 'Source Control' + }) + ) + expect(mocks.captureParityScreen).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + deviceUdid: 'simulator', + screenshotName: 'hosted-review-portrait.png', + title: 'Changes' + }) + ) + expect(result.parityFixture).toEqual({ + sourceControl: { screen: 'source-control' }, + review: { screen: 'review' } + }) + }) + it('falls back to measured points when WebKit omits accessibility descendants', async () => { mocks.tapAccessibilityControl.mockRejectedValue(new Error('missing descendant')) diff --git a/mobile/src/mobile-web/hosted-ios-source-control-review-parity.test.ts b/mobile/src/mobile-web/hosted-ios-source-control-review-parity.test.ts new file mode 100644 index 00000000000..399ef38c58c --- /dev/null +++ b/mobile/src/mobile-web/hosted-ios-source-control-review-parity.test.ts @@ -0,0 +1,121 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + captureScreenshot: vi.fn(), + compareScreenshots: vi.fn(), + dismissDeveloperMenu: vi.fn(), + longPressControlByPrefix: vi.fn(), + readTextPoint: vi.fn(), + tapControl: vi.fn(), + tapControlStartingWith: vi.fn(), + waitForControl: vi.fn(), + waitForControlByPrefix: vi.fn(), + waitForControlEndingWith: vi.fn(), + waitForControlStartingWith: vi.fn() +})) + +vi.mock('node:child_process', () => ({ + execFile: mocks.captureScreenshot +})) +vi.mock('../../scripts/emulator-developer-menu-dismissal.mjs', () => ({ + dismissEmulatorDeveloperMenuIfPresent: mocks.dismissDeveloperMenu +})) +vi.mock('../../scripts/hosted-ios-emulator-accessibility.mjs', () => ({ + tapHostedIosAccessibilityControl: mocks.tapControl, + tapHostedIosAccessibilityControlStartingWith: mocks.tapControlStartingWith, + waitForHostedIosAccessibilityControl: mocks.waitForControl, + waitForHostedIosAccessibilityControlByLabelPrefix: mocks.waitForControlByPrefix, + waitForHostedIosAccessibilityControlEndingWith: mocks.waitForControlEndingWith, + waitForHostedIosAccessibilityControlStartingWith: mocks.waitForControlStartingWith +})) +vi.mock('../../scripts/hosted-ios-emulator-long-press.mjs', () => ({ + longPressHostedIosAccessibilityControlByLabelPrefix: mocks.longPressControlByPrefix +})) +vi.mock('../../scripts/hosted-ios-screenshot-parity.mjs', () => ({ + assertHostedIosScreenshotParity: mocks.compareScreenshots +})) +vi.mock('../../scripts/hosted-webview-cdp-session.mjs', () => ({ + readHostedWebViewTextPoint: mocks.readTextPoint +})) + +import { + captureHostedSourceControlReviewScreen, + captureNativeSourceControlReviewBaselines +} from '../../scripts/hosted-ios-source-control-review-parity.mjs' + +describe('hosted iOS Source Control and Review parity', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.captureScreenshot.mockImplementation((_command, _args, callback) => + callback(null, '', '') + ) + mocks.compareScreenshots.mockResolvedValue({ changedPixelRatio: 0.01 }) + mocks.longPressControlByPrefix.mockResolvedValue({ x: 0.5, y: 0.4 }) + mocks.readTextPoint.mockResolvedValue({ x: 0.2, y: 0.1 }) + mocks.tapControl.mockResolvedValue({ x: 0.2, y: 0.1 }) + mocks.tapControlStartingWith.mockResolvedValue({ x: 0.5, y: 0.5 }) + mocks.waitForControl.mockResolvedValue({ x: 0.2, y: 0.1 }) + mocks.waitForControlByPrefix.mockResolvedValue({ x: 0.5, y: 0.4 }) + mocks.waitForControlEndingWith.mockResolvedValue({ x: 0.5, y: 0.4 }) + mocks.waitForControlStartingWith.mockResolvedValue({ x: 0.5, y: 0.5 }) + }) + + it('captures the real host-origin Source Control and standalone Review path', async () => { + const baselines = await captureNativeSourceControlReviewBaselines({ + deviceUdid: 'simulator', + emulator: { deviceUdid: 'simulator' }, + expectedWorkspace: 'mobile-rearch', + runtimeDirectory: '/tmp/parity', + timeoutMs: 30_000 + }) + + expect(mocks.longPressControlByPrefix).toHaveBeenCalledWith( + { deviceUdid: 'simulator' }, + 'mobile-rearch', + 30_000 + ) + expect(mocks.tapControl).toHaveBeenCalledWith( + { deviceUdid: 'simulator' }, + 'Source Control', + 30_000 + ) + expect(mocks.tapControlStartingWith).toHaveBeenCalledWith( + { deviceUdid: 'simulator' }, + 'Open changed file ', + 30_000 + ) + expect(baselines).toEqual({ + review: { + screenTitlePoint: { x: 0.2, y: 0.1 }, + screenshot: '/tmp/parity/native-review-portrait.png' + }, + sourceControl: { + screenTitlePoint: { x: 0.2, y: 0.1 }, + screenshot: '/tmp/parity/native-source-control-portrait.png' + } + }) + }) + + it('compares a hosted route against its native screenshot and title landmark', async () => { + const capture = await captureHostedSourceControlReviewScreen({ + deviceUdid: 'simulator', + document: { href: 'orca-mobile-web://build/h/host/review/worktree' }, + nativeBaseline: { + screenTitlePoint: { x: 0.2, y: 0.1 }, + screenshot: '/tmp/parity/native-review-portrait.png' + }, + runtimeDirectory: '/tmp/parity', + screenshotName: 'hosted-review-portrait.png', + title: 'Changes', + timeoutMs: 30_000 + }) + + expect(capture.screenshotParity).toEqual({ changedPixelRatio: 0.01 }) + expect(mocks.compareScreenshots).toHaveBeenCalledWith({ + hostedLandmark: { x: 0.2, y: 0.1 }, + hostedScreenshot: '/tmp/parity/hosted-review-portrait.png', + nativeLandmark: { x: 0.2, y: 0.1 }, + nativeScreenshot: '/tmp/parity/native-review-portrait.png' + }) + }) +}) diff --git a/mobile/src/mobile-web/mobile-web-source-control-compare-page.ts b/mobile/src/mobile-web/mobile-web-source-control-compare-page.ts new file mode 100644 index 00000000000..495c61a3d53 --- /dev/null +++ b/mobile/src/mobile-web/mobile-web-source-control-compare-page.ts @@ -0,0 +1,44 @@ +import { Buffer } from 'buffer' +import { sha256 } from '@noble/hashes/sha256' +import { + MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES, + type MobileWebSourceControlCompareEntry +} from '../../../src/shared/mobile-web/source-control-history-contract' +import { MobileWebBrokerError } from './mobile-web-broker-error' + +export function mobileWebCompareEntryPage( + snapshot: Record, + allEntries: MobileWebSourceControlCompareEntry[], + offset: number, + limit: number +): MobileWebSourceControlCompareEntry[] { + const entries = allEntries.slice(offset, offset + limit) + while ( + entries.length > 0 && + encodedByteLength({ ...snapshot, entries }) > + MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES - 8 * 1024 + ) { + entries.pop() + } + if ( + offset < allEntries.length && + (entries.length === 0 || + encodedByteLength({ ...snapshot, entries }) > + MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES - 8 * 1024) + ) { + throw new MobileWebBrokerError('too_large') + } + return entries +} + +export function mobileWebBranchCompareRevision( + snapshot: Record, + entries: MobileWebSourceControlCompareEntry[] +): string { + const content = new TextEncoder().encode(JSON.stringify({ snapshot, entries })) + return Buffer.from(sha256(content)).toString('hex') +} + +function encodedByteLength(value: unknown): number { + return new TextEncoder().encode(JSON.stringify(value)).byteLength +} diff --git a/mobile/src/mobile-web/mobile-web-source-control-history-operations.test.ts b/mobile/src/mobile-web/mobile-web-source-control-history-operations.test.ts index 8934e571dd7..c027179b3a8 100644 --- a/mobile/src/mobile-web/mobile-web-source-control-history-operations.test.ts +++ b/mobile/src/mobile-web/mobile-web-source-control-history-operations.test.ts @@ -160,6 +160,7 @@ describe('mobile web source-control history operations', () => { expect('entries' in compare && compare.entries.length).toBeLessThan( MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT ) + expect('nextOffset' in compare && compare.nextOffset).not.toBeNull() expect('truncated' in compare && compare.truncated).toBe(true) }) @@ -206,11 +207,89 @@ describe('mobile web source-control history operations', () => { baseRef: 'main', changedFiles: entries.length, commitsAhead: 3, + totalEntries: entries.length, truncated: true }) expect(result.entries).toHaveLength(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT) + expect(result.nextOffset).toBe(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT) expect(JSON.stringify(result)).not.toContain('/private/repository') expect(JSON.stringify(result)).not.toContain('errorMessage') + + const continuation = await executeMobileWebSourceControlHistoryOperation({ + operation: 'branchCompare', + payload: { + workspaceId: 'workspace-1', + baseRef: 'main', + offset: result.nextOffset, + expectedRevision: result.revision + }, + client, + workspaceAuthority + }) + if (!('baseOid' in continuation)) { + throw new Error('Expected branch comparison continuation') + } + expect(continuation).toMatchObject({ + revision: result.revision, + offset: MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT, + entries: [{ relativePath: `src/file-${entries.length - 1}.ts` }], + nextOffset: null + }) + }) + + it('rejects a branch comparison continuation after the host revision changes', async () => { + const entries = Array.from( + { length: MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT + 1 }, + (_, index) => ({ path: `src/file-${index}.ts`, status: 'modified' }) + ) + const client = rpcClient({ + summary: { + baseOid: OID_A, + compareRef: 'feature/mobile', + headOid: OID_B, + mergeBase: OID_A, + changedFiles: entries.length, + status: 'ready' + }, + entries + }) + const first = await executeMobileWebSourceControlHistoryOperation({ + operation: 'branchCompare', + payload: { workspaceId: 'workspace-1', baseRef: 'main' }, + client, + workspaceAuthority + }) + if (!('baseOid' in first) || first.nextOffset === null) { + throw new Error('Expected branch comparison page') + } + vi.mocked(client.sendRequest).mockResolvedValueOnce({ + ok: true, + result: { + summary: { + baseOid: OID_A, + compareRef: 'feature/mobile', + headOid: 'c'.repeat(40), + mergeBase: OID_A, + changedFiles: entries.length, + status: 'ready' + }, + entries + } + }) + + await expect( + executeMobileWebSourceControlHistoryOperation({ + operation: 'branchCompare', + payload: { + workspaceId: 'workspace-1', + baseRef: 'main', + offset: first.nextOffset, + expectedRevision: first.revision + }, + client, + workspaceAuthority + }) + ).rejects.toMatchObject({ code: 'conflict' }) }) it('requires a full commit ID and returns request-bound comparison identity', async () => { diff --git a/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts b/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts index dede245a7f5..1d7fd146ae0 100644 --- a/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts +++ b/mobile/src/mobile-web/mobile-web-source-control-history-operations.ts @@ -76,7 +76,7 @@ export async function executeMobileWebSourceControlHistoryOperation(args: { if (!response.ok) { throw new MobileWebBrokerError('host_error') } - return sanitizeMobileWebBranchCompare(response.result, payload.workspaceId, payload.baseRef) + return sanitizeMobileWebBranchCompare(response.result, payload) } const payload = MobileWebSourceControlCommitComparePayloadSchema.parse(args.payload) diff --git a/mobile/src/mobile-web/mobile-web-source-control-history-sanitizers.ts b/mobile/src/mobile-web/mobile-web-source-control-history-sanitizers.ts index 3866af2e298..be9980d96d6 100644 --- a/mobile/src/mobile-web/mobile-web-source-control-history-sanitizers.ts +++ b/mobile/src/mobile-web/mobile-web-source-control-history-sanitizers.ts @@ -1,6 +1,7 @@ import { MOBILE_WEB_SOURCE_CONTROL_BRANCH_LIMIT, MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT, + MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES, MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES, MOBILE_WEB_SOURCE_CONTROL_HISTORY_RESPONSE_MAX_BYTES, MobileWebGitObjectIdSchema, @@ -11,6 +12,7 @@ import { MobileWebSourceControlCompareEntrySchema, MobileWebSourceControlHistoryResultSchema, type MobileWebSourceControlBranchCompareResult, + type MobileWebSourceControlBranchComparePayload, type MobileWebSourceControlBranchesResult, type MobileWebSourceControlCommitCompareResult, type MobileWebSourceControlCompareEntry, @@ -18,6 +20,10 @@ import { type MobileWebSourceControlHistoryResult } from '../../../src/shared/mobile-web/source-control-history-contract' import { MobileWebBrokerError } from './mobile-web-broker-error' +import { + mobileWebBranchCompareRevision, + mobileWebCompareEntryPage +} from './mobile-web-source-control-compare-page' import { sanitizeMobileWebHistoryItem, sanitizeMobileWebHistoryRef @@ -99,19 +105,18 @@ export function sanitizeMobileWebHistory( export function sanitizeMobileWebBranchCompare( result: unknown, - workspaceId: string, - baseRef: string + payload: MobileWebSourceControlBranchComparePayload ): MobileWebSourceControlBranchCompareResult { const summary = compareSummary(result) - const sanitized = sanitizeCompareEntries(result) + const sanitized = sanitizeCompareEntries(result, MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES) const changedFiles = Math.max( sanitized.reportedCount, safeNonnegativeInteger(summary.changedFiles) ) const commitsAhead = optionalNonnegativeInteger(summary.commitsAhead) - return MobileWebSourceControlBranchCompareResultSchema.parse({ - workspaceId, - baseRef, + const snapshot = { + workspaceId: payload.workspaceId, + baseRef: payload.baseRef, compareRef: boundedString(summary.compareRef, 240) ?? 'HEAD', baseOid: sanitizeObjectId(summary.baseOid), headOid: sanitizeObjectId(summary.headOid), @@ -119,8 +124,30 @@ export function sanitizeMobileWebBranchCompare( changedFiles, ...(commitsAhead === undefined ? {} : { commitsAhead }), status: branchCompareStatus(summary.status), - entries: sanitized.entries, + totalEntries: sanitized.entries.length, truncated: sanitized.truncated || changedFiles > sanitized.entries.length + } + const revision = mobileWebBranchCompareRevision(snapshot, sanitized.entries) + if (payload.expectedRevision && payload.expectedRevision !== revision) { + throw new MobileWebBrokerError('conflict') + } + const entries = mobileWebCompareEntryPage( + snapshot, + sanitized.entries, + payload.offset, + payload.limit + ) + const nextOffset = + payload.offset + entries.length < sanitized.entries.length + ? payload.offset + entries.length + : null + return MobileWebSourceControlBranchCompareResultSchema.parse({ + ...snapshot, + revision, + offset: payload.offset, + entries, + nextOffset, + truncated: snapshot.truncated || nextOffset !== null }) } @@ -130,7 +157,11 @@ export function sanitizeMobileWebCommitCompare( commitId: string ): MobileWebSourceControlCommitCompareResult { const summary = compareSummary(result) - const sanitized = sanitizeCompareEntries(result) + const sanitized = sanitizeCompareEntries( + result, + MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT, + true + ) const changedFiles = Math.max( sanitized.reportedCount, safeNonnegativeInteger(summary.changedFiles) @@ -149,7 +180,11 @@ export function sanitizeMobileWebCommitCompare( }) } -function sanitizeCompareEntries(result: unknown): { +function sanitizeCompareEntries( + result: unknown, + limit: number, + enforceResponseBudget = false +): { entries: MobileWebSourceControlCompareEntry[] reportedCount: number truncated: boolean @@ -158,32 +193,29 @@ function sanitizeCompareEntries(result: unknown): { throw new MobileWebBrokerError('host_error') } const entries: MobileWebSourceControlCompareEntry[] = [] - let retainedBytes = 0 let droppedByBudget = false - for (const candidate of result.entries.slice(0, MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT)) { + for (const candidate of result.entries.slice(0, limit)) { const entry = sanitizeCompareEntry(candidate) if (!entry) { continue } - const nextBytes = encodedByteLength(entry) + 1 if ( - retainedBytes + nextBytes > - MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES - 8 * 1024 + enforceResponseBudget && + encodedByteLength([...entries, entry]) > + MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES - 8 * 1024 ) { droppedByBudget = true break } - retainedBytes += nextBytes entries.push(entry) } return { entries, reportedCount: result.entries.length, truncated: - result.entries.length > MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT || + result.entries.length > limit || droppedByBudget || - entries.length < - Math.min(result.entries.length, MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT) + entries.length < Math.min(result.entries.length, limit) } } diff --git a/mobile/src/session/web-host-diff-review-client.test.ts b/mobile/src/session/web-host-diff-review-client.test.ts index e5bd4b39cd3..dc6e5cac2d2 100644 --- a/mobile/src/session/web-host-diff-review-client.test.ts +++ b/mobile/src/session/web-host-diff-review-client.test.ts @@ -105,6 +105,75 @@ describe('web host diff review client', () => { ).resolves.toMatchObject({ ok: false, error: { message: 'Source control action failed' } }) }) + it('assembles every revision-bound branch comparison page for the unchanged Review UI', async () => { + const bridge = bridgeClient() + const firstEntries = Array.from({ length: 128 }, (_, index) => ({ + relativePath: `src/file-${index}.ts`, + status: 'modified' + })) + bridge.sourceControlBranchCompare = vi + .fn() + .mockResolvedValueOnce(branchComparePage(firstEntries, 0, 128)) + .mockResolvedValueOnce( + branchComparePage([{ relativePath: 'src/file-128.ts', status: 'added' }], 128, null) + ) + const client = webHostDiffReviewClient( + bridge as unknown as MobileWebBridgeClient, + 'workspace-1' + ) + + const response = await client.sendRequest('git.branchCompare', { + worktree: 'id:workspace-1', + baseRef: 'main' + }) + + expect(response).toMatchObject({ + ok: true, + result: { + summary: { baseRef: 'main', changedFiles: 129 }, + entries: [ + { path: 'src/file-0.ts', status: 'modified' }, + ...Array.from({ length: 127 }, (_, index) => ({ + path: `src/file-${index + 1}.ts`, + status: 'modified' + })), + { path: 'src/file-128.ts', status: 'added' } + ] + } + }) + expect(bridge.sourceControlBranchCompare).toHaveBeenNthCalledWith(2, { + workspaceId: 'workspace-1', + baseRef: 'main', + offset: 128, + limit: 128, + expectedRevision: revision + }) + }) + + it('rejects a stale branch comparison page instead of mixing Review queues', async () => { + const bridge = bridgeClient() + bridge.sourceControlBranchCompare = vi + .fn() + .mockResolvedValueOnce( + branchComparePage([{ relativePath: 'src/app.ts', status: 'modified' }], 0, 1) + ) + .mockResolvedValueOnce({ + ...branchComparePage([{ relativePath: 'src/next.ts', status: 'modified' }], 1, null), + revision: 'b'.repeat(64) + }) + const client = webHostDiffReviewClient( + bridge as unknown as MobileWebBridgeClient, + 'workspace-1' + ) + + await expect( + client.sendRequest('git.branchCompare', { + worktree: 'id:workspace-1', + baseRef: 'main' + }) + ).resolves.toMatchObject({ ok: false, error: { message: 'Source control action failed' } }) + }) + it('maps shell metadata, session tabs, diff opening, and terminal send locally', async () => { const bridge = bridgeClient() const client = webHostDiffReviewClient( @@ -232,6 +301,7 @@ function bridgeClient() { reviewState: payload.reviewState }) ), + sourceControlBranchCompare: vi.fn(), sourceControlReviewDiff: vi.fn(), sourceControlReviewOpen: vi.fn().mockResolvedValue(null), sourceControlReviewTerminalSend: vi.fn().mockResolvedValue({ accepted: true }), @@ -260,6 +330,29 @@ function bridgeClient() { } } +function branchComparePage( + entries: { relativePath: string; status: 'modified' | 'added' }[], + offset: number, + nextOffset: number | null +) { + return { + workspaceId: 'workspace-1', + baseRef: 'main', + compareRef: 'HEAD', + baseOid: 'a'.repeat(40), + headOid: 'b'.repeat(40), + mergeBase: 'a'.repeat(40), + changedFiles: 129, + status: 'ready' as const, + revision, + offset, + totalEntries: 129, + entries, + nextOffset, + truncated: false + } +} + function diffRow( index: number, kind: 'context' | 'add' | 'delete', diff --git a/mobile/src/source-control/web-host-provider-review-creation.ts b/mobile/src/source-control/web-host-provider-review-creation.ts index a494b82aa83..dce7457fc21 100644 --- a/mobile/src/source-control/web-host-provider-review-creation.ts +++ b/mobile/src/source-control/web-host-provider-review-creation.ts @@ -1,12 +1,23 @@ import type { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' +import type { MobileWebProviderReviewEligibilityResult } from '../../../src/shared/mobile-web/provider-review-creation-contract' type RequestParams = Record +export type WebHostProviderReviewEligibilityCache = { + pending: Map> + settled: Map +} + +export function createWebHostProviderReviewEligibilityCache(): WebHostProviderReviewEligibilityCache { + return { pending: new Map(), settled: new Map() } +} + export async function handleWebHostProviderReviewCreation(args: { client: MobileWebBridgeClient workspaceId: string method: string params: RequestParams + eligibilityCache: WebHostProviderReviewEligibilityCache }): Promise { if ( args.method !== 'hostedReview.getCreationEligibility' && @@ -28,11 +39,14 @@ export async function handleWebHostProviderReviewCreation(args: { expectedBranch: status.branch } if (args.method === 'hostedReview.getCreationEligibility') { - const result = await args.client.providerReviewCreationEligibility({ - ...identity, - ...(args.params.base === null || args.params.base === undefined - ? {} - : { base: requiredString(args.params.base) }) + const result = await loadWebHostProviderReviewEligibility({ + client: args.client, + identity, + base: + args.params.base === null || args.params.base === undefined + ? undefined + : requiredString(args.params.base), + cache: args.eligibilityCache }) const { workspaceId: _workspaceId, @@ -73,6 +87,53 @@ export const WEB_HOST_PROVIDER_REVIEW_CREATION_UNHANDLED = Symbol( 'provider-review-creation-unhandled' ) +export async function loadWebHostProviderReviewEligibility(args: { + client: MobileWebBridgeClient + identity: { + workspaceId: string + expectedHead: string + expectedBranch: string + } + base?: string + cache: WebHostProviderReviewEligibilityCache +}): Promise { + const key = `${args.identity.expectedHead}\0${args.identity.expectedBranch}\0${args.base ?? ''}` + const pending = args.cache.pending.get(key) + if (pending) { + return pending + } + const request = args.client.providerReviewCreationEligibility({ + ...args.identity, + ...(args.base === undefined ? {} : { base: args.base }) + }) + args.cache.pending.set(key, request) + try { + const result = await request + if (args.base === undefined) { + args.cache.settled.clear() + args.cache.settled.set(key, result) + } + return result + } finally { + if (args.cache.pending.get(key) === request) { + args.cache.pending.delete(key) + } + } +} + +export function takeWebHostProviderReviewEligibility( + cache: WebHostProviderReviewEligibilityCache, + identity: { + expectedHead: string + expectedBranch: string + } +): MobileWebProviderReviewEligibilityResult | null { + const key = `${identity.expectedHead}\0${identity.expectedBranch}\0` + const result = cache.settled.get(key) ?? null + cache.settled.delete(key) + return result +} + function requiredProvider( value: unknown ): 'github' | 'gitlab' | 'bitbucket' | 'azure-devops' | 'gitea' { diff --git a/mobile/src/source-control/web-host-provider-review-loader.ts b/mobile/src/source-control/web-host-provider-review-loader.ts new file mode 100644 index 00000000000..667b921c63a --- /dev/null +++ b/mobile/src/source-control/web-host-provider-review-loader.ts @@ -0,0 +1,81 @@ +import type { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' +import type { MobileWebProviderReviewResult } from '../../../src/shared/mobile-web/provider-review-contract' +import { + loadWebHostProviderReviewEligibility, + takeWebHostProviderReviewEligibility, + type WebHostProviderReviewEligibilityCache +} from './web-host-provider-review-creation' + +export type WebHostProviderReviewCache = { + key: string | null + result: MobileWebProviderReviewResult | null + commentIds: Map +} + +export function createWebHostProviderReviewCache(): WebHostProviderReviewCache { + return { key: null, result: null, commentIds: new Map() } +} + +export async function readWebHostGitHubRepositoryEligibility( + client: MobileWebBridgeClient, + workspaceId: string, + cache: WebHostProviderReviewEligibilityCache +) { + const status = await client.sourceControlStatus({ workspaceId, limit: 1 }) + if (!status.head || !status.branch) { + throw new Error('conflict') + } + const eligibility = await loadWebHostProviderReviewEligibility({ + client, + identity: { + workspaceId, + expectedHead: status.head, + expectedBranch: status.branch + }, + cache + }) + return eligibility.provider === 'github' ? { owner: 'paired-host', repo: 'workspace' } : null +} + +export async function loadWebHostProviderReview( + client: MobileWebBridgeClient, + workspaceId: string, + cache: WebHostProviderReviewCache, + eligibilityCache: WebHostProviderReviewEligibilityCache +): Promise { + const status = await client.sourceControlStatus({ workspaceId, limit: 64 }) + if (!status.head || !status.branch) { + throw new Error('conflict') + } + const key = `${status.head}\0${status.branch}` + if (cache.key === key && cache.result) { + return cache.result + } + const eligibility = takeWebHostProviderReviewEligibility(eligibilityCache, { + expectedHead: status.head, + expectedBranch: status.branch + }) + if (eligibility?.review === null) { + const result = { + workspaceId, + observedHead: status.head, + branch: status.branch, + review: null + } + cache.key = key + cache.result = result + return result + } + const result = await client.providerReview({ + workspaceId, + expectedHead: status.head, + expectedBranch: status.branch + }) + cache.key = key + cache.result = result + cache.commentIds.clear() + result.review?.comments.forEach((comment, index) => { + cache.commentIds.set(index + 1, comment.id) + }) + return result +} diff --git a/mobile/src/source-control/web-host-provider-review-requests.test.ts b/mobile/src/source-control/web-host-provider-review-requests.test.ts index bb22606ab6d..dbd81c0a655 100644 --- a/mobile/src/source-control/web-host-provider-review-requests.test.ts +++ b/mobile/src/source-control/web-host-provider-review-requests.test.ts @@ -6,6 +6,78 @@ const WORKSPACE_ID = 'workspace-page-1' const HEAD = 'a'.repeat(40) describe('web host provider review requests', () => { + it('probes GitHub repository eligibility without requiring an existing review', async () => { + const bridge = bridgeClient() + bridge.providerReview.mockResolvedValue({ + workspaceId: WORKSPACE_ID, + observedHead: HEAD, + branch: 'main', + review: null + }) + const client = webHostSourceControlClient( + bridge as unknown as MobileWebBridgeClient, + WORKSPACE_ID + ) + + const response = await client.sendRequest('github.repoSlug', {}) + + expect(response).toMatchObject({ + ok: true, + result: { owner: 'paired-host', repo: 'workspace' } + }) + expect(bridge.providerReview).not.toHaveBeenCalled() + expect(bridge.providerReviewCreationEligibility).toHaveBeenCalledWith({ + workspaceId: WORKSPACE_ID, + expectedHead: HEAD, + expectedBranch: 'main' + }) + }) + + it('coalesces the PR chip and creation-action eligibility probes', async () => { + const bridge = bridgeClient() + bridge.providerReviewCreationEligibility.mockResolvedValue({ + workspaceId: WORKSPACE_ID, + observedHead: HEAD, + branch: 'main', + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'auth_required', + nextAction: 'authenticate', + reviewLookupOutcome: 'unavailable' + }) + const client = webHostSourceControlClient( + bridge as unknown as MobileWebBridgeClient, + WORKSPACE_ID + ) + + const [slug, eligibility] = await Promise.all([ + client.sendRequest('github.repoSlug', {}), + client.sendRequest('hostedReview.getCreationEligibility', {}) + ]) + + expect(slug).toMatchObject({ + ok: true, + result: { owner: 'paired-host', repo: 'workspace' } + }) + expect(eligibility).toMatchObject({ + ok: true, + result: { provider: 'github', blockedReason: 'auth_required' } + }) + expect(bridge.providerReviewCreationEligibility).toHaveBeenCalledTimes(1) + + const review = await client.sendRequest('hostedReview.forBranch', { + branch: 'main' + }) + + expect(review).toMatchObject({ ok: true, result: null }) + await expect(client.sendRequest('github.prForBranch', {})).resolves.toMatchObject({ + ok: true, + result: null + }) + expect(bridge.providerReview).not.toHaveBeenCalled() + }) + it('projects one provider review into the unchanged PR presentation', async () => { const bridge = bridgeClient() const client = webHostSourceControlClient( @@ -189,6 +261,17 @@ function bridgeClient() { allowedSubmissionActions: ['comment'] } }), + providerReviewCreationEligibility: vi.fn().mockResolvedValue({ + workspaceId: WORKSPACE_ID, + observedHead: HEAD, + branch: 'main', + provider: 'github', + review: null, + canCreate: false, + blockedReason: 'dirty', + nextAction: 'commit', + reviewLookupOutcome: 'not_found' + }), providerMutateReview: vi.fn(), providerManageReview: vi.fn(), providerReviewQuery: vi.fn().mockResolvedValue({ diff --git a/mobile/src/source-control/web-host-provider-review-requests.ts b/mobile/src/source-control/web-host-provider-review-requests.ts index 8a6b822ae4d..72c266cad61 100644 --- a/mobile/src/source-control/web-host-provider-review-requests.ts +++ b/mobile/src/source-control/web-host-provider-review-requests.ts @@ -3,6 +3,13 @@ import type { MobileWebProviderReview, MobileWebProviderReviewResult } from '../../../src/shared/mobile-web/provider-review-contract' +import type { WebHostProviderReviewEligibilityCache } from './web-host-provider-review-creation' +import { + createWebHostProviderReviewCache, + loadWebHostProviderReview, + readWebHostGitHubRepositoryEligibility, + type WebHostProviderReviewCache +} from './web-host-provider-review-loader' import { handleWebHostProviderReviewMutation, WEB_HOST_PROVIDER_REVIEW_MUTATION_METHODS @@ -10,15 +17,7 @@ import { type RequestParams = Record -export type WebHostProviderReviewCache = { - key: string | null - result: MobileWebProviderReviewResult | null - commentIds: Map -} - -export function createWebHostProviderReviewCache(): WebHostProviderReviewCache { - return { key: null, result: null, commentIds: new Map() } -} +export { createWebHostProviderReviewCache, type WebHostProviderReviewCache } export async function handleWebHostProviderReviewRequest(args: { client: MobileWebBridgeClient @@ -26,21 +25,20 @@ export async function handleWebHostProviderReviewRequest(args: { method: string params: RequestParams cache: WebHostProviderReviewCache + eligibilityCache: WebHostProviderReviewEligibilityCache }): Promise { - const { client, workspaceId, method, params, cache } = args + const { client, workspaceId, method, params, cache, eligibilityCache } = args if ( !PROVIDER_READ_METHODS.has(method) && !WEB_HOST_PROVIDER_REVIEW_MUTATION_METHODS.has(method) ) { return WEB_HOST_PROVIDER_REVIEW_UNHANDLED } - const loaded = await loadProviderReview(client, workspaceId, cache) - const review = loaded.review if (method === 'github.repoSlug') { - return review && review.provider !== 'github' - ? null - : { owner: 'paired-host', repo: 'workspace' } + return readWebHostGitHubRepositoryEligibility(client, workspaceId, eligibilityCache) } + const loaded = await loadWebHostProviderReview(client, workspaceId, cache, eligibilityCache) + const review = loaded.review if (method === 'hostedReview.forBranch') { return review ? hostedReviewSummary(review) : null } @@ -83,33 +81,6 @@ const PROVIDER_READ_METHODS = new Set([ 'github.listAssignableUsers' ]) -async function loadProviderReview( - client: MobileWebBridgeClient, - workspaceId: string, - cache: WebHostProviderReviewCache -): Promise { - const status = await client.sourceControlStatus({ workspaceId, limit: 64 }) - if (!status.head || !status.branch) { - throw new Error('conflict') - } - const key = `${status.head}\0${status.branch}` - if (cache.key === key && cache.result) { - return cache.result - } - const result = await client.providerReview({ - workspaceId, - expectedHead: status.head, - expectedBranch: status.branch - }) - cache.key = key - cache.result = result - cache.commentIds.clear() - result.review?.comments.forEach((comment, index) => { - cache.commentIds.set(index + 1, comment.id) - }) - return result -} - function hostedReviewSummary(review: MobileWebProviderReview) { return { provider: review.provider, diff --git a/mobile/src/source-control/web-host-source-control-client.ts b/mobile/src/source-control/web-host-source-control-client.ts index 6a731ff95cd..3eb480dbd3e 100644 --- a/mobile/src/source-control/web-host-source-control-client.ts +++ b/mobile/src/source-control/web-host-source-control-client.ts @@ -18,6 +18,7 @@ import { WEB_HOST_PROVIDER_REVIEW_UNHANDLED } from './web-host-provider-review-requests' import { + createWebHostProviderReviewEligibilityCache, handleWebHostProviderReviewCreation, WEB_HOST_PROVIDER_REVIEW_CREATION_UNHANDLED } from './web-host-provider-review-creation' @@ -27,6 +28,7 @@ export function webHostSourceControlClient( workspaceId: string ): RpcClient { const providerReviewCache = createWebHostProviderReviewCache() + const providerReviewEligibilityCache = createWebHostProviderReviewEligibilityCache() return { async sendRequest(method, input) { if (!matchesBoundWorkspace(input, workspaceId)) { @@ -46,7 +48,8 @@ export function webHostSourceControlClient( client: bridgeClient, workspaceId, method, - params + params, + eligibilityCache: providerReviewEligibilityCache }) if (creation !== WEB_HOST_PROVIDER_REVIEW_CREATION_UNHANDLED) { return creation @@ -56,7 +59,8 @@ export function webHostSourceControlClient( workspaceId, method, params, - cache: providerReviewCache + cache: providerReviewCache, + eligibilityCache: providerReviewEligibilityCache }) if (provider !== WEB_HOST_PROVIDER_REVIEW_UNHANDLED) { return provider diff --git a/mobile/src/source-control/web-host-source-control-reads.ts b/mobile/src/source-control/web-host-source-control-reads.ts index 1b5b8ed655d..05d5ea29bd4 100644 --- a/mobile/src/source-control/web-host-source-control-reads.ts +++ b/mobile/src/source-control/web-host-source-control-reads.ts @@ -1,4 +1,9 @@ import type { MobileWebBridgeClient } from '../../../src/mobile-web/src/mobile-web-bridge-client' +import { + MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT, + MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES, + type MobileWebSourceControlBranchCompareResult +} from '../../../src/shared/mobile-web/source-control-history-contract' import { MOBILE_WEB_SOURCE_CONTROL_STATUS_LIMIT } from '../../../src/shared/mobile-web/source-control-operation-contract' type RequestParams = Record @@ -55,7 +60,7 @@ export async function readWebHostSourceControlRequest(args: { } if (method === 'git.branchCompare') { const baseRef = requiredString(params.baseRef) - return branchCompareResult(await client.sourceControlBranchCompare({ workspaceId, baseRef })) + return branchCompareResult(await readWebHostBranchCompare(client, workspaceId, baseRef)) } if (method === 'git.commitCompare') { const commitId = requiredString(params.commitId) @@ -104,6 +109,52 @@ function branchCompareResult( } } +async function readWebHostBranchCompare( + client: MobileWebBridgeClient, + workspaceId: string, + baseRef: string +): Promise { + const entries: MobileWebSourceControlBranchCompareResult['entries'] = [] + let offset = 0 + let expectedRevision: string | undefined + let expectedTotalEntries: number | undefined + for (;;) { + const page = await client.sourceControlBranchCompare({ + workspaceId, + baseRef, + offset, + limit: MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT, + ...(expectedRevision ? { expectedRevision } : {}) + }) + if (expectedRevision && page.revision !== expectedRevision) { + throw new Error('conflict') + } + expectedRevision = page.revision + if ( + (expectedTotalEntries !== undefined && page.totalEntries !== expectedTotalEntries) || + entries.length + page.entries.length > MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES || + entries.length + page.entries.length > page.totalEntries + ) { + throw new Error('invalid_message') + } + expectedTotalEntries = page.totalEntries + entries.push(...page.entries) + if (page.nextOffset === null) { + if (entries.length !== expectedTotalEntries) { + throw new Error('invalid_message') + } + return { ...page, offset: 0, totalEntries: entries.length, entries } + } + if ( + page.nextOffset !== offset + page.entries.length || + page.nextOffset > MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES + ) { + throw new Error('invalid_message') + } + offset = page.nextOffset + } +} + function commitCompareResult( result: Awaited> ) { diff --git a/src/mobile-web/src/mobile-web-source-control-repository.test.tsx b/src/mobile-web/src/mobile-web-source-control-repository.test.tsx index cfee0a548de..8c73031c470 100644 --- a/src/mobile-web/src/mobile-web-source-control-repository.test.tsx +++ b/src/mobile-web/src/mobile-web-source-control-repository.test.tsx @@ -25,7 +25,7 @@ describe('MobileWebSourceControlRepository', () => { fireEvent.click(screen.getByRole('button', { name: 'Compare feature/mobile' })) expect(await screen.findByText('src/branch.ts')).toBeDefined() expect(client.sourceControlBranchCompare).toHaveBeenCalledWith( - { workspaceId: 'workspace-1', baseRef: 'feature/mobile' }, + { workspaceId: 'workspace-1', baseRef: 'feature/mobile', offset: 0, limit: 128 }, expect.objectContaining({ signal: expect.any(AbortSignal) }) ) diff --git a/src/mobile-web/src/mobile-web-source-control-request-client.test.ts b/src/mobile-web/src/mobile-web-source-control-request-client.test.ts index 3d34e19e56e..96e296b8a1c 100644 --- a/src/mobile-web/src/mobile-web-source-control-request-client.test.ts +++ b/src/mobile-web/src/mobile-web-source-control-request-client.test.ts @@ -134,7 +134,9 @@ describe('mobile web source-control request client', () => { const branchHarness = createHarness() const branch = branchHarness.client.sourceControlBranchCompare({ workspaceId: 'workspace-1', - baseRef: 'main' + baseRef: 'main', + offset: 0, + limit: 128 }) branchHarness.client.receive( response('A'.repeat(22), { @@ -146,7 +148,11 @@ describe('mobile web source-control request client', () => { mergeBase: 'a'.repeat(40), changedFiles: 0, status: 'ready', + revision: 'c'.repeat(64), + offset: 0, + totalEntries: 0, entries: [], + nextOffset: null, truncated: false }) ) diff --git a/src/mobile-web/src/mobile-web-source-control-request-client.ts b/src/mobile-web/src/mobile-web-source-control-request-client.ts index 2ac841236d2..0c4462c6f1b 100644 --- a/src/mobile-web/src/mobile-web-source-control-request-client.ts +++ b/src/mobile-web/src/mobile-web-source-control-request-client.ts @@ -142,7 +142,11 @@ export class MobileWebSourceControlRequestClient { options ) .then((result) => { - if (result.baseRef !== payload.baseRef) { + if ( + result.baseRef !== payload.baseRef || + result.offset !== payload.offset || + (payload.expectedRevision && result.revision !== payload.expectedRevision) + ) { throw new MobileWebBridgeClientError('invalid_message', false) } return matchingIdentity(payload, result) diff --git a/src/mobile-web/src/use-mobile-web-source-control-repository.ts b/src/mobile-web/src/use-mobile-web-source-control-repository.ts index 6ce5d01b966..83b88d1627c 100644 --- a/src/mobile-web/src/use-mobile-web-source-control-repository.ts +++ b/src/mobile-web/src/use-mobile-web-source-control-repository.ts @@ -131,7 +131,7 @@ export function useMobileWebSourceControlRepository(args: { const request = selection.kind === 'branch' ? client.sourceControlBranchCompare( - { workspaceId, baseRef: selection.id }, + { workspaceId, baseRef: selection.id, offset: 0, limit: 128 }, { signal: controller.signal } ) : client.sourceControlCommitCompare( diff --git a/src/shared/mobile-web/source-control-history-contract.test.ts b/src/shared/mobile-web/source-control-history-contract.test.ts index 5650a0a0478..6b07d91942b 100644 --- a/src/shared/mobile-web/source-control-history-contract.test.ts +++ b/src/shared/mobile-web/source-control-history-contract.test.ts @@ -89,6 +89,9 @@ describe('mobile web source-control history contract', () => { mergeBase: OID, changedFiles: MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT, status: 'ready', + revision: 'c'.repeat(64), + offset: 0, + totalEntries: MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT, entries: Array.from( { length: MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT }, (_, index) => ({ @@ -96,6 +99,7 @@ describe('mobile web source-control history contract', () => { status: 'modified' }) ), + nextOffset: null, truncated: false } expect(MobileWebSourceControlBranchCompareResultSchema.safeParse(result).success).toBe(true) diff --git a/src/shared/mobile-web/source-control-history-contract.ts b/src/shared/mobile-web/source-control-history-contract.ts index 2d84a4a9893..17c14d47838 100644 --- a/src/shared/mobile-web/source-control-history-contract.ts +++ b/src/shared/mobile-web/source-control-history-contract.ts @@ -10,6 +10,7 @@ export const MOBILE_WEB_SOURCE_CONTROL_HISTORY_MAX_LIMIT = 100 export const MOBILE_WEB_SOURCE_CONTROL_HISTORY_PARENT_LIMIT = 16 export const MOBILE_WEB_SOURCE_CONTROL_HISTORY_REFERENCE_LIMIT = 32 export const MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT = 128 +export const MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES = 4_000 export const MOBILE_WEB_SOURCE_CONTROL_HISTORY_RESPONSE_MAX_BYTES = 192 * 1024 export const MOBILE_WEB_SOURCE_CONTROL_COMPARE_RESPONSE_MAX_BYTES = 192 * 1024 @@ -96,7 +97,18 @@ export const MobileWebSourceControlHistoryResultSchema = z export const MobileWebSourceControlBranchComparePayloadSchema = z .object({ workspaceId: MobileWebWorkspaceIdSchema, - baseRef: MobileWebGitRefNameSchema + baseRef: MobileWebGitRefNameSchema, + offset: z.number().int().min(0).max(MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES).default(0), + limit: z + .number() + .int() + .min(1) + .max(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT) + .default(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT), + expectedRevision: z + .string() + .regex(/^[a-f0-9]{64}$/) + .optional() }) .strict() @@ -128,9 +140,18 @@ export const MobileWebSourceControlBranchCompareResultSchema = z changedFiles: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER), commitsAhead: z.number().int().nonnegative().max(Number.MAX_SAFE_INTEGER).optional(), status: z.enum(['ready', 'invalid-base', 'unborn-head', 'no-merge-base', 'error']), + revision: z.string().regex(/^[a-f0-9]{64}$/), + offset: z.number().int().min(0).max(MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES), + totalEntries: z.number().int().min(0).max(MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES), entries: z .array(MobileWebSourceControlCompareEntrySchema) .max(MOBILE_WEB_SOURCE_CONTROL_COMPARE_ENTRY_LIMIT), + nextOffset: z + .number() + .int() + .min(1) + .max(MOBILE_WEB_SOURCE_CONTROL_COMPARE_MAX_ENTRIES) + .nullable(), truncated: z.boolean() }) .strict()