Skip to content

test(e2e): fix paired HTML preview link spec after preview links began honoring in-app preference - #39190

Merged
nwparker merged 1 commit into
mainfrom
nwparker/nightly-e2e-paired-remote-html-preview-local-render
Nov 6, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/nightly-e2e-paired-remote-html-preview-local-render

Conversation

@nwparker

@nwparker nwparker commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

A nightly end-to-end test clicks a link inside a previewed HTML file on a paired (remote) workspace and expects it to open in Orca's browser. It has failed every night since Oct 31 because the app changed on purpose and the test didn't.

Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​42 $\color{#cf222e}{\Huge{\mathbf{−}}}$​39 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10

What Changed

Before: after #33622, a confirmed preview link follows the user's "Open links in Orca" preference. That preference is off by default, so the link goes to the system browser, and the app no longer calls openBrowserProfileTabInActiveWorkspace, the store action the test was spying on. Nothing recorded the click and no tab opened, so the test timed out on "a confirmed preview link never opened an Orca browser tab".

After (test only):

  • The paired client turns on openLinksInApp before the link checks, which is the setting that now decides where the link goes.
  • The stale store spy (and its as never cast) is replaced by readClientBrowserPageUrls, which reads the URLs of the client's browser pages directly. So the test now checks where the link actually ends up, whichever opener created the tab.
  • Every assertion is at least as strong as before. With no input, and after Cancel, the client's page URLs must match the baseline exactly. After "Open link", a client page must be at https://example.com/from-preview, a new tab must exist, and the preview must still be open.
  • Adds a PR e2e route so that changes to doc-preview sources (src/**/doc-preview*, browser-pane/workspace-doc/*, and the inventory helper) run this spec, plus routing tests.

Why

The product change was deliberate: #33622 made preview links follow browser preferences and modifier keys, and unit tests cover the system-browser default. Turning the preference on is what a user who wants in-app links would do. Running the default path in e2e would launch a real system browser on CI and on dev machines. Spying on the new opener would tie the test to another internal function. Reading the resulting page URL tests what the user actually sees.

Linked Issue

Nightly e2e failures (runs 41223095376 through 42273799189).

Visual Proof

N/A: test-only change.

Testing

  • Local run gets through the whole preview, chip, close and reopen flow up to the native-focus step. That step is blocked under ORCA_BACKGROUND_LAUNCH by design, so the link-click section runs only on CI.
  • pnpm test config/scripts/workflow/source-routing/pr-e2e-gate-contract.test.mjs, pnpm tc, check:code-quality:changed
  • The changed spec is selected for this PR's e2e run.

Risk

Level: Low. Only test code and CI routing change. The new route adds this spec (about 4 minutes) to PRs that touch doc-preview sources.

Agent skill upstream boundary

  • Not applicable.

Notes

The link opens on the paired runtime because the source is runtime-owned, so this checks the remote path rather than a local-only one.

…nk preference

Since #33622 a confirmed HTML preview link honors openLinksInApp (default off,
so it opens the system browser) and no longer calls
openBrowserProfileTabInActiveWorkspace, so the spec's store spy never fired.
Opt the client into in-app links and observe the opened page URL directly.
Route doc-preview sources to the spec.
@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Walkthrough

The changes add a source route for the paired HTML preview E2E spec, with contract tests for files that should and should not select it. The spec now uses client browser page URLs to check that genuine input and cancellation do not open a page, and that confirming a link opens the external URL. The inventory helper adds URL collection across workspaces and removes the prior link-routing instrumentation.

Merge Risk: Moderate · up to 4762b

The updated paired-preview test cannot complete its link checks under the required launch policy, and its URL checks can pass without proving the intended behavior. Resolve those test gaps before relying on the new routing and assertions for merge readiness.

Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title clearly describes the paired HTML preview test fix.
Description check Passed The description covers the change, rationale, testing, and risk. It does not state the tested platforms or rollback plan. It also does not link an issue, which the template requires for outside contri…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description covers the change, rationale, testing, and risk. It does not state the tested platforms or rollback plan. It also does not link an issue, which the template requires for outside contributors.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 4 files.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Nitpick comments: 1

tests/e2e/paired-remote-html-preview-local-render.spec.ts: 1

520-524: Trivial · Functional Correctness · Poor tradeoff

Distinguish a canceled open from the confirmed open.

If Cancel incorrectly starts an asynchronous open, the tab can appear after the single URL check. The later check can accept that tab as the confirmed open. Observe the Cancel case through an appropriate completion boundary before attributing a tab to confirmation.

Review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8742eba5-ecd4-461e-8276-27ade755bd3b
Commits

Reviewing files that changed from the base of the PR and between be3b87f and 4762b2c.

Files selected for processing: 4

  • config/scripts/workflow/source-routing/pr-browser-e2e-source-routes.mjs
  • config/scripts/workflow/source-routing/pr-e2e-gate-contract.test.mjs
  • tests/e2e/helpers/paired-html-preview-inventory.ts
  • tests/e2e/paired-remote-html-preview-local-render.spec.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Comment on lines +190 to 195
export async function readClientBrowserPageUrls(page: Page): Promise<string[]> {
return page.evaluate(() =>
Object.values(window.__store?.getState().browserPagesByWorkspace ?? {}).flatMap((pages) =>
pages.map((browserPage) => browserPage.url)
)
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Functional Correctness · Quick win

Supported by static analysis

Script executed:

git diff bf07b8f60eca8c44b083fd60396a40bdde3d046c 4762b2c3b12832d8d7dd0046a0c333cef0acdd23 -- tests/e2e/helpers/paired-html-preview-inventory.ts tests/e2e/paired-remote-html-preview-local-render.spec.ts
printf '\n--- URL bindings ---\n'
rg -n -F -e 'openBrowserProfileTabInActiveWorkspace' -e 'browserPagesByWorkspace' -e 'EXTERNAL_LINK_URL' -- tests/e2e/paired-remote-html-preview-local-render.spec.ts tests/e2e/helpers/paired-html-preview-inventory.ts src/renderer/src/store/slices/

Repository: stablyai/orca

Length of output: 24923


Script executed:

printf '%s\n' '--- browser tab actions ---'
nl -ba src/renderer/src/store/slices/browser/browser-tab-actions.ts | sed -n '1,280p'
printf '%s\n' '--- page creation actions ---'
nl -ba src/renderer/src/store/slices/browser/browser-page-create-actions.ts | sed -n '1,120p'
printf '%s\n' '--- fixture and link assertion ---'
nl -ba tests/e2e/paired-remote-html-preview-local-render.spec.ts | sed -n '20,150p;450,615p'
printf '%s\n' '--- supporting URL constructors ---'
rg -n -F -e 'createBrowserPage' -e 'normalizeBrowserUrl' -e 'normalizeUrl' -- src/renderer/src/store/slices/browser src/renderer/src/store/slices/browser-page-records.ts

Repository: stablyai/orca

Length of output: 36516


Script executed:

printf '%s\n' '--- page record URL construction ---'
nl -ba src/renderer/src/store/slices/browser-page-records.ts | sed -n '1,100p'
printf '%s\n' '--- page URL update ---'
nl -ba src/renderer/src/store/slices/browser/browser-page-state-actions.ts | sed -n '160,215p'
printf '%s\n' '--- preview routing bindings ---'
rg -n -F -e 'openLinksInApp' -e 'Open link to' -- src/renderer/src src/main | grep -iE 'preview|link|external|browser' 
printf '%s\n' '--- test initialization ---'
nl -ba tests/e2e/paired-remote-html-preview-local-render.spec.ts | sed -n '150,260p'

Repository: stablyai/orca

Length of output: 28880


Script executed:

printf '%s\n' '--- confirmation binding ---'
nl -ba src/renderer/src/components/browser-pane/workspace-doc/doc-preview-external-link-confirmation.tsx | sed -n '1,180p'
printf '%s\n' '--- routing implementation ---'
nl -ba src/renderer/src/lib/browser/navigation/http-link-routing.ts | sed -n '1,270p'
printf '%s\n' '--- redaction definition ---'
rg -n -F 'redactKagiSessionToken' src/shared/browser-url.ts
nl -ba src/shared/browser-url.ts | sed -n '1,180p'

Repository: stablyai/orca

Length of output: 21799


Script executed:

printf '%s\n' '--- redaction function ---'
nl -ba src/shared/browser-url.ts | sed -n '174,205p'
printf '%s\n' '--- workspace opener registration ---'
rg -n -F 'registerWorkspaceHttpLinkBrowserOpener' src/renderer/src
printf '%s\n' '--- remote link browser opening ---'
rg -n -F -e 'allowClientBrowserFallback' -e 'registerWorkspaceHttpLinkBrowserOpener(' -- src/renderer/src/runtime src/renderer/src/lib/browser/navigation

Repository: stablyai/orca

Length of output: 5094


Script executed:

nl -ba src/renderer/src/store/index.ts | sed -n '35,80p;150,205p'
rg -n -F -e 'openWorkspaceHttpLinkBrowser' -e 'openRuntimeBrowserTab' -e 'allowClientBrowserFallback' -- src/renderer/src/store/index.ts src/renderer/src/runtime src/renderer/src/lib/workspace src/renderer/src/store | head -100

Repository: stablyai/orca

Length of output: 6333


Script executed:

printf '%s\n' '--- opener declarations ---'
rg -n -F -e 'openWorkspaceBrowserTabForLink' -e 'request.url' -e 'createBrowserTab(' -e 'allowClientBrowserFallback' src/renderer/src/lib/browser/workspace/workspace-browser-tab-open*
printf '%s\n' '--- opener source ---'
nl -ba src/renderer/src/lib/browser/workspace/workspace-browser-tab-open.ts | sed -n '1,260p'

Repository: stablyai/orca

Length of output: 13349


Script executed:

printf '%s\n' '--- client link creation ---'
rg -n -F -e 'openClientWorkspaceBrowserLink' -e 'createBrowserTab' -e 'url:' src/renderer/src/lib/browser/workspace/workspace-browser-link-open.ts
nl -ba src/renderer/src/lib/browser/workspace/workspace-browser-link-open.ts | sed -n '1,250p'
printf '%s\n' '--- paired runtime creation locations ---'
rg -n -F -e 'createWebRuntimeSessionBrowserTab' -e 'url:' -e 'createBrowserTab' src/renderer/src/runtime/web-runtime-browser-creation.ts
nl -ba src/renderer/src/runtime/web-runtime-browser-creation.ts | sed -n '1,240p'

Repository: stablyai/orca

Length of output: 20510


Script executed:

rg -n -F -e 'stageWebRuntimeBrowserCreation' -e 'args.url' -e 'createBrowserTab(' src/renderer/src/runtime/web-runtime-browser-creation-context.ts src/renderer/src/runtime/web-runtime-browser-tab-staging.ts src/renderer/src/lib/browser/navigation/browser-link-open.ts
nl -ba src/renderer/src/runtime/web-runtime-browser-creation-context.ts | sed -n '1,200p'
nl -ba src/renderer/src/runtime/web-runtime-browser-tab-staging.ts | sed -n '1,190p'
nl -ba src/renderer/src/lib/browser/navigation/browser-link-open.ts | sed -n '1,200p'

Repository: stablyai/orca

Length of output: 27566


Script executed:

printf '%s\n' '--- test setup and routing assumptions ---'
nl -ba tests/e2e/paired-remote-html-preview-local-render.spec.ts | sed -n '260,450p'
printf '%s\n' '--- inventory workspace counting ---'
nl -ba tests/e2e/helpers/paired-html-preview-inventory.ts | sed -n '1,120p'
printf '%s\n' '--- fixture browser settings ---'
rg -n -F -e 'browserClientHostedRemoteEnabled' -e 'managed-browser' -e 'browser.headless' -e 'startClientHostedMarkerFixture' -- tests/e2e/paired-remote-html-preview-local-render.spec.ts tests/e2e/helpers/client-hosted-browser-fixture.ts src/renderer/src/lib/client-creation-action-policy*

Repository: stablyai/orca

Length of output: 16657


Script executed:

printf '%s\n' '--- client creation provider policy ---'
nl -ba src/renderer/src/lib/client-creation-action-policy.ts | sed -n '1,190p'
printf '%s\n' '--- creation failure handling ---'
nl -ba src/renderer/src/runtime/web-runtime-browser-creation-failure.ts | sed -n '1,230p'
printf '%s\n' '--- paired launch defaults ---'
rg -n -F -e 'launchPairedElectronClient' -e 'browserClientHostedRemoteEnabled' tests/e2e/helpers/paired* tests/e2e/helpers/orca-app.ts src/shared/global-settings-defaults*

Repository: stablyai/orca

Length of output: 13875


Wait for the new page to finish creation.

On the paired-runtime path, the staged page already has EXTERNAL_LINK_URL. The assertion can pass before the create RPC fails and removes that page. Require successful materialization before accepting its URL.

Comment on lines 485 to 488
expect(await focusPairedClientWindow(client)).toMatchObject({
isFocused: true,
isVisible: true
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major · Maintainability & Code Quality · Heavy lift

Supported by static analysis

Script executed:

git diff bf07b8f60eca8c44b083fd60396a40bdde3d046c 4762b2c3b12832d8d7dd0046a0c333cef0acdd23 -- tests/e2e/paired-remote-html-preview-local-render.spec.ts
printf '\n--- relevant references ---\n'
rg -n -F -e 'focusPairedClientWindow' -e 'ORCA_BACKGROUND_LAUNCH' -e 'test.skip' -e 'process.env.CI' -- tests/e2e/paired-remote-html-preview-local-render.spec.ts tests/e2e/helpers
printf '\n--- test context ---\n'
nl -ba tests/e2e/paired-remote-html-preview-local-render.spec.ts | sed -n '1,180p;350,550p'

Repository: stablyai/orca

Length of output: 27092


Script executed:

printf '%s\n' '--- focus helper ---'
nl -ba tests/e2e/helpers/paired-client-window-reveal.ts
printf '%s\n' '--- paired client launch environment ---'
nl -ba tests/e2e/helpers/paired-client-runtime-environment.ts | sed -n '1,150p'
printf '%s\n' '--- test and helper references ---'
rg -n -F -e 'paired-remote-html-preview-local-render.spec' -e 'focusPairedClientWindow' -e 'ORCA_E2E_PAIRED' -- tests config .github playwright.config.ts 2>/dev/null
printf '%s\n' '--- test end and cleanup ---'
nl -ba tests/e2e/paired-remote-html-preview-local-render.spec.ts | sed -n '530,640p'

Repository: stablyai/orca

Length of output: 40808


Script executed:

printf '%s\n' '--- paired launch references ---'
rg -n -F -e 'launchPairedElectronClient' -e 'ORCA_BACKGROUND_LAUNCH' -e 'electron.launch' -- tests/e2e/helpers/paired-electron-client.ts tests/e2e/helpers/orca-app.ts tests/playwright.config.ts
printf '%s\n' '--- paired client implementation ---'
nl -ba tests/e2e/helpers/paired-electron-client.ts | sed -n '1,260p'
printf '%s\n' '--- test config guard and selection ---'
nl -ba tests/playwright.config.ts | sed -n '1,240p'
printf '%s\n' '--- fixture launch environment ---'
rg -n -C 5 -F -e 'ORCA_BACKGROUND_LAUNCH' -e 'test.skip' -- tests/e2e/helpers/orca-app.ts

Repository: stablyai/orca

Length of output: 13495


Script executed:

printf '%s\n' '--- home environment construction ---'
rg -n -F -e 'createElectronHomeIsolation' -e 'inheritedEnv' -e 'launchEnv' -- tests/e2e/helpers/electron-home-isolation.ts
nl -ba tests/e2e/helpers/electron-home-isolation.ts | sed -n '1,180p'
printf '%s\n' '--- intervening test control flow ---'
nl -ba tests/e2e/paired-remote-html-preview-local-render.spec.ts | sed -n '175,355p'
printf '%s\n' '--- fixture guards and launch ---'
nl -ba tests/e2e/helpers/orca-app.ts | sed -n '100,220p'

Repository: stablyai/orca

Length of output: 21013


Make the link test compatible with the background launch policy.

With ORCA_BACKGROUND_LAUNCH=1, the focus helper throws before the changed link assertions run. Removing that setting lets the helper reveal and focus the window, but this spec is not an approved exception. Exercise the link flow without violating the launch policy.

Source: Coding guidelines

@nwparker
nwparker merged commit a1650f7 into main Nov 6, 2026
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant