Harden e2e setup/teardown cleanup and scope the BCV nav helper to the top toolbar

e2e-tests/fixtures/app.fixture.ts

@@ -12,6 +12,7 @@ import {
   preConfigureSettings,
   PROCESS_READY_TIMEOUT,
   teardownElectronApp,
+  describePageError,
 } from './helpers';
 
 export { expect } from '@playwright/test';
@@ -65,7 +66,7 @@ export const test = base.extend<TestAppFixtures, WorkerAppFixtures>({
      *
      * @param err The error thrown in the page context.
      */
-    const onPageError = (err: Error) => console.error(`Page error: ${err.message}`);
+    const onPageError = (err: Error) => console.error(`Page error: ${describePageError(err)}`);
 
     /**
      * Log console error messages from the page to the process console.

e2e-tests/fixtures/helpers.ts

@@ -78,6 +78,33 @@ const SERVICE_HOST_OBJECT_METHODS = [
 const FATAL_STARTUP_PAGE_ERROR =
   /Timeout reached when waiting for .*allThemeFamiliesById to settle/i;
 
+/**
+ * Build a detailed description of a renderer page error for logging and pattern matching.
+ * Playwright surfaces uncaught page errors as `Error` objects, but when the page throws a non-Error
+ * value (e.g. a plain object) `.message` collapses to something useless like "Object" — hiding the
+ * real cause from both the CI log and {@link FATAL_STARTUP_PAGE_ERROR}, which is why a doomed cold
+ * start can burn its whole readiness budget instead of fast-failing. This combines the message, the
+ * stack (which often carries the true text for wrapped throwables), and a JSON dump of any
+ * own-enumerable properties so the real error survives to the log and to the tripwire regex.
+ *
+ * @param err The error surfaced by Playwright's `pageerror` event.
+ * @returns A newline-joined description containing every distinct detail recoverable from `err`.
+ */
+export function describePageError(err: Error): string {
+  const parts: string[] = [];
+  if (err.message) parts.push(err.message);
+  if (err.stack) parts.push(err.stack);
+  try {
+    // A real Error serializes to "{}" — its message and stack are non-enumerable — so only a wrapped
+    // non-Error throwable yields properties worth keeping; skip the empty/null cases.
+    const json = JSON.stringify(err);
+    if (json && json !== '{}' && json !== 'null') parts.push(json);
+  } catch {
+    // Circular/non-serializable value — the message and stack above are our best description.
+  }
+  return parts.join('\n');
+}
+
 /**
  * Keep in sync with GET_METHODS from @shared/data/rpc.model. Required to be 'rpc.discover' by the
  * OpenRPC specification.
@@ -177,6 +204,13 @@ export const E2E_SETTINGS_OVERRIDES: Record<string, unknown> = {
  * the `.cdp-*` run-marker files). Shared by BOTH the smoke and CDP tiers, not one file per tier:
  * there is only one `settings.json` to protect, so a stale backup left by either tier's crashed run
  * can be recovered by the other tier's own self-heal (see {@link backupAndSeedSettings}).
+ *
+ * Sharing one file is safe only because seed/restore cycles never overlap: both Playwright configs
+ * set `workers: 1` and `fullyParallel: false`, and the tiers run sequentially. Two concurrent
+ * cycles would interleave — one restoring `settings.json` to the original while the other's app
+ * reads it at startup (dropping the overrides), and the backup could capture an already-seeded
+ * file. If either tier ever gains workers or the two are run at once, give each tier its own backup
+ * file.
  */
 const SETTINGS_BACKUP_FILE = path.join(__dirname, '..', '.e2e-settings-backup');
 
@@ -194,6 +228,11 @@ const SETTINGS_ABSENT_SENTINEL = '__SETTINGS_ABSENT__';
  * @param overrides Setting keys to merge into the file (e.g. `{ 'platform.firstRunComplete': true
  *   }`).
  * @returns Nothing.
+ * @throws A filesystem error while backing up the original settings, creating the settings
+ *   directory, or writing the seeded file. The seed may be partially applied when it throws, so a
+ *   caller that has to leave the developer's settings untouched must roll it back with
+ *   {@link restoreBackedUpSettings} (see {@link preConfigureSettings}). A corrupt settings file is
+ *   not an error — it is overwritten with just the overrides.
  */
 export function backupAndSeedSettings(overrides: Record<string, unknown>): void {
   restoreBackedUpSettings();
@@ -252,9 +291,21 @@ export function restoreBackedUpSettings(): void {
  * @returns A restore function that puts the file back to its exact pre-call contents (or deletes it
  *   if it did not exist). Call it AFTER the app has closed so the developer's saved settings are
  *   not permanently replaced by test values.
+ * @throws Whatever {@link backupAndSeedSettings} throws (a filesystem error while backing up,
+ *   creating the settings directory, or writing the seeded file). The caller never receives a
+ *   restore function in that case, so this rolls the partial seed back itself before rethrowing —
+ *   otherwise a mid-seed failure would strand the developer's settings in a seeded state.
  */
 export function preConfigureSettings(overrides: Record<string, unknown>): () => void {
-  backupAndSeedSettings(overrides);
+  try {
+    backupAndSeedSettings(overrides);
+  } catch (error) {
+    // Best-effort and non-throwing (see restoreBackedUpSettings), so this cannot mask `error`. If
+    // the backup file was written before the failure, this restores from it; if the failure came
+    // before that, there is nothing to undo and it is a no-op.
+    restoreBackedUpSettings();
+    throw error;
+  }
   return restoreBackedUpSettings;
 }
 
@@ -621,6 +672,13 @@ interface DockTabTitlesOptions {
  * It wraps the WHOLE readiness sequence (service-host wait AND dock-tab wait) because the fatal
  * theme-settle error can surface during either and never self-recovers.
  *
+ * Deliberately NOT a tripwire signal: the unhandled `PlatformError` rejections paranext-core emits
+ * during boot (e.g. the `platform.getOSPlatform` -32601 not-found race). Tripping on those was
+ * tried and reverted — launches that emit them frequently still come up healthy, and because that
+ * boot race recurs on fresh launches, treating them as fatal fast-failed every retry of a
+ * recoverable start and turned flaky-but-green runs into hard failures. Do not promote a startup
+ * signal to fatal unless launches that emit it have been observed to NEVER recover.
+ *
  * The listener is registered only when `enabled` (a warm shared instance leaves it off, so a stale
  * error from a long-past cold start can't abort an otherwise-healthy wait), and always removed in
  * `finally` so it can't leak across tests on the shared CDP page. A sentinel distinguishes
@@ -645,9 +703,13 @@ export async function withFatalStartupTripwire<T>(
   const fatalErrorTripped = new Promise<never>((_resolve, reject) => {
     if (!enabled) return;
     onFatalPageError = (err: Error) => {
-      if (!FATAL_STARTUP_PAGE_ERROR.test(err.message)) return;
-      fatalError.message = err.message;
-      reject(new Error(err.message));
+      // Match against the full description, not just `err.message`: a non-Error throwable can carry
+      // the theme-settle text in its stack while its `.message` is a useless "Object", which would
+      // otherwise slip past the tripwire and cost this launch its whole readiness budget.
+      const described = describePageError(err);
+      if (!FATAL_STARTUP_PAGE_ERROR.test(described)) return;
+      fatalError.message = described;
+      reject(new Error(described));
     };
     page.on('pageerror', onFatalPageError);
   });
@@ -1354,11 +1416,24 @@ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise<void>
  * submits it. Requires a fully-qualified reference (book, chapter, and verse — e.g. `"GEN 1:1"`);
  * partial references are ambiguous and are not auto-submitted by the control.
  *
- * The toolbar's book-chapter control is only ENABLED when BCV navigation has a resolved target web
- * view. In the default simple mode that target is the first open Scripture editor with a project,
- * NOT the focused Interlinearizer tab — so the control needs an open, project-bearing Scripture
- * editor to be drivable at all. That precondition holds because callers run this after
- * `ensureInterlinearizerOpenOnWeb`, which loads the WEB project into a Scripture Editor.
+ * Drives the TOP TOOLBAR's book-chapter control specifically. In Power mode — which
+ * {@link E2E_SETTINGS_OVERRIDES} forces for every launch — it is not the only one on the page:
+ * paranext-core also renders a PER-TAB `BookChapterControl` in each web view's own tab toolbar, and
+ * every instance carries the same `aria-label="book-chapter-trigger"`. So the trigger is scoped to
+ * the toolbar's `toolbar-reserved-space-wrapper` root rather than selected page-wide; a bare
+ * `.first()` would depend on the toolbar happening to render before the dock layout in document
+ * order, and would silently drive some tab's control if paranext-core ever reordered them.
+ *
+ * The scoping also matters for the enabled-wait below, because the two kinds of control differ:
+ * only the toolbar's is passed a `disabled` prop (bound to whether BCV navigation has a resolved
+ * target web view). The per-tab controls receive no such prop and are therefore ALWAYS enabled, so
+ * waiting on one of those would make the wait vacuous — it would pass even with no navigation
+ * target resolved.
+ *
+ * That target is the first open Scripture editor with a project, NOT the focused Interlinearizer
+ * tab — so the toolbar control needs an open, project-bearing Scripture editor to be drivable at
+ * all. That precondition holds because callers run this after `ensureInterlinearizerOpenOnWeb`,
+ * which loads the WEB project into a Scripture Editor.
  *
  * The control can be momentarily disabled right after startup, before the target resolves, so this
  * waits a BOUNDED time for it to enable before clicking; a plain click on a still-disabled button
@@ -1373,7 +1448,13 @@ export async function ensureInterlinearizerOpenOnWeb(page: Page): Promise<void>
  *   if it is enabled but its popover does not open or close within the timeouts.
  */
 export async function navigateToScriptureRef(page: Page, reference: string): Promise<void> {
-  const trigger = page.locator('button[aria-label="book-chapter-trigger"]').first();
+  // Scope to the top toolbar's own root: in Power mode each open web view tab renders its own
+  // identically-labeled (and always-enabled) BookChapterControl, so a page-wide lookup would be
+  // ambiguous. `.first()` stays as a strict-mode guard within that single scope.
+  const trigger = page
+    .getByTestId('toolbar-reserved-space-wrapper')
+    .locator('button[aria-label="book-chapter-trigger"]')
+    .first();
 
   // Bounded wait for the control to become enabled, covering the brief post-launch window before the
   // navigation target resolves. `toBeEnabled` polls (unlike `isEnabled`, which reads once); swallow
@@ -1385,11 +1466,11 @@ export async function navigateToScriptureRef(page: Page, reference: string): Pro
     .catch(() => false);
   if (!enabled) {
     throw new Error(
-      `navigateToScriptureRef: the platform book-chapter control never became enabled, so ` +
+      `navigateToScriptureRef: the top toolbar's book-chapter control never became enabled, so ` +
         `"${reference}" could not be navigated to. This means BCV navigation has no resolved ` +
-        'target: in the default simple interface mode the target is the first open Scripture editor ' +
-        'with a project, so the WEB Scripture Editor is likely closed or its project failed to load. ' +
-        'Ensure it is open (openInterlinearizerFromScriptureEditor) before navigating.',
+        'target: the target is the first open Scripture editor with a project, so the WEB Scripture ' +
+        'Editor is likely closed or its project failed to load. Ensure it is open ' +
+        '(openInterlinearizerFromScriptureEditor) before navigating.',
     );
   }