Bypass paranext-core first-run wizard overlay in e2e tests

[imnasnainaec]

This is an e2e-harness-only catch-up fix for recent paranext-core behavior changes, plus reliability hardening for the harness itself. No production/extension code changes.

Problem

Against current paranext-core, the e2e suite can't reliably reach the dock/Home UI it drives:

• First-run wizard overlay. On a fresh profile, when the registration service is unreachable (dev machines and CI), paranext-core renders a full-screen modal ("Couldn't verify your registration") that intercepts pointer events, so every test that drives the toolbar/menus times out behind it.
• Simple interface mode hides dock tabs. platform.interfaceMode defaults to 'simple', which hides the Home/Editor tab bars entirely and populates the Resources column with default tabs that never resolve real titles on a bare e2e profile — breaking both the cold-start readiness gate and the WEB-tab click flow.
• Fresh profiles now open directly to Home, not an empty "Scripture Editor" tab as the suite assumed, so the project-opening flow never got far enough to reach it.
• The bundled sample WEB project installs asynchronously, independent of dock/extension readiness, so a test could reach Home before it was registered. Most costly on the CDP tier, where one shared instance serves the whole run: losing this race once fails every downstream feature test.

Fix

Settings seeding. E2E_SETTINGS_OVERRIDES (platform.firstRunComplete: true, platform.interfaceMode: 'power') is written into paranext-core's dev-appdata/data/settings.json before Electron launches, in both tiers — the smoke tier's per-worker fixture (preConfigureSettings()) and the CDP tier's global setup (backupAndSeedSettings()). Forcing Power mode is a stopgap, not a permanent choice — see Follow-up. The original file is backed up to disk (not just in memory) and self-heals a stale backup left by a prior hard-killed run, so a CI timeout or Ctrl+C never loses a developer's real settings; it's restored once the app closes (smoke) or the launched instance is killed (CDP).

Flexible dock-layout handling. openInterlinearizerFromScriptureEditor() and ensureInterlinearizerOpenOnWeb() (fixtures/helpers.ts) now accept either starting layout — an empty Scripture Editor tab with no Home tab, or an already-open Home tab — and skip the redundant toolbar Home-button click when Home is already open.

WEB-project readiness. waitForAtLeastOneProjectMetadata() (mirrors paranext-core's own e2e helper) polls ProjectLookupService.getMetadataForAllProjects until a project is registered. It's called inline, right before clicking WEB's row in Home, and also once in the CDP tier's global setup, alongside the interlinearizer extension's own activation gate (waitForInterlinearizerReady()). Since the CDP tier launches one shared instance for the whole run, gating setup on both means a broken or slow-starting instance fails setup once with a clear message, instead of every downstream feature test burning its own timeout.

CDP setup failure-path cleanup. If any readiness wait throws (ports never open, renderer never settles, extension never activates, project never registers), global setup now kills the launched process tree, removes the isolated user-data dir, restores the seeded settings, and removes its PID/user-data marker files before rethrowing. Playwright skips globalTeardown when globalSetup throws, so previously all of that leaked to the next run.

Teardown/process-utility hardening. Shared helpers in process-utils.ts — waitForProcessExit (race a live exit event against a timeout), waitForPidExit (poll a bare PID for exit, for callers with no live process handle), removeDirWithRetry (retry directory removal once after a delay) — replace duplicated one-off sleep/retry logic in both teardown files. killProcessFromPidFile now returns { killed, pid } instead of a bare boolean, so callers can wait on that specific PID's exit before touching files it may still hold open.

Full local e2e suite (smoke + CDP) passes against current paranext-core.

Follow-up

Filed #170 to add real Simple-mode e2e coverage instead of permanently forcing Power mode.

---

🤖 Generated with Claude Code

Devin: https://app.devin.ai/review/sillsdev/interlinearizer-extension/pull/169

---

This change is

Summary by CodeRabbit

• Bug Fixes
  • Improved end-to-end startup readiness and interlinearizer activation detection (including Home dock tab variations).
  • Made end-to-end settings overrides deterministic: pre-applied before launch and restored on warm reuse and on failures.
  • Hardened teardown reliability with better process-exit waiting and resilient cleanup for isolated app/user-data directories.
• Chores
  • Added an additional end-to-end settings backup to the ignore list.
  • Standardized PID-file handling for more consistent dev-server cleanup.

[coderabbitai]

Warning

Review limit reached

@imnasnainaec, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6c689300-b46d-4b91-9ea6-d19901343145

📥 Commits

Reviewing files that changed from the base of the PR and between 9fd3c7c and e4c753e.

📒 Files selected for processing (1)

• e2e-tests/global-teardown-cdp.ts

📝 Walkthrough

Walkthrough

The E2E infrastructure now seeds and restores settings around Electron runs, centralizes process and directory cleanup helpers, improves CDP failure handling, and supports alternate Home-tab startup layouts for interlinearizer readiness.

Changes

E2E lifecycle management

Layer / File(s)	Summary
Shared cleanup primitives e2e-tests/process-utils.ts	Adds bounded process-exit waits, PID polling, and retryable directory removal utilities.
Settings seeding and fixture lifecycle e2e-tests/fixtures/helpers.ts, e2e-tests/fixtures/app.fixture.ts, .gitignore	Backs up, seeds, and restores E2E settings around Electron fixture execution, with guaranteed teardown and ignored backup artifacts.
CDP startup and cleanup e2e-tests/global-setup-cdp.ts, e2e-tests/global-teardown-cdp.ts	Seeds settings before CDP launch and restores them across startup failure, warm-instance reuse, and teardown while coordinating process and user-data cleanup.
PID wiring and startup readiness e2e-tests/global-setup.ts, e2e-tests/global-teardown.ts, e2e-tests/fixtures/helpers.ts	Centralizes the renderer PID-file path, exposes structured PID termination results, and accepts Home-tab startup layouts during interlinearizer readiness checks.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

• sillsdev/interlinearizer-extension#170 — Directly concerns E2E_SETTINGS_OVERRIDES and settings seeding in the E2E launch flows.

Possibly related PRs

• sillsdev/interlinearizer-extension#151 — Both changes modify E2E interlinearizer startup and readiness helpers.

Suggested reviewers: alex-rawlings-yyc

Sequence Diagram(s)

sequenceDiagram
  participant PlaywrightFixture
  participant Electron
  participant ProcessUtils
  participant CDPTeardown
  PlaywrightFixture->>Electron: seed settings and launch
  Electron-->>PlaywrightFixture: expose test UI
  PlaywrightFixture->>ProcessUtils: wait for app exit and remove user data
  CDPTeardown->>Electron: kill process by PID
  CDPTeardown->>ProcessUtils: wait for PID exit and remove user data
  CDPTeardown->>PlaywrightFixture: restore backed-up settings

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: seeding e2e settings to bypass the first-run wizard overlay.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch e2e-first-run-overlay

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[alex-rawlings-yyc]

@alex-rawlings-yyc reviewed 8 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on imnasnainaec).

---

e2e-tests/global-teardown-cdp.ts line 49 at r2 (raw file):

  }

  restoreSeededSettings();

⛏️ Not ownership-gated, unlike everything else in this teardown

This runs even after a warm-instance-reuse run, which seeds nothing. Since the backup file is deliberately shared across both tiers (helpers.ts:176-180), a reuse-path teardown can consume and delete a backup belonging to a smoke run — leaving that run's seeded test values as the developer's "restored" settings.

Every other resource here is gated on the marker files; this is the one that isn't. Suggested fix:

const launchedApp = fs.existsSync(CDP_PID_FILE) || fs.existsSync(CDP_USER_DATA_FILE);

then if (launchedApp) restoreSeededSettings();

Two details that aren't obvious: the snapshot has to be taken before killProcessFromPidFile, which unlinks CDP_PID_FILE — read it after and ownership always reports false. And || rather than &&, so a run that crashed between the two marker writes still restores the seed it owns.

Low severity — the tiers run sequentially under npm run test:e2e, so this needs an interleaving to bite. Raising it because it's a one-line guard, and because the app kill and dir removal directly above both gate on the marker files. Setup's clearStaleOwnershipMarkers() comment says clearing them "keeps teardown a true no-op on this reuse path" — which this line quietly breaks.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

e2e-tests/global-teardown-cdp.ts (1)

35-58: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Ensure shared teardown runs when CDP cleanup fails.

The readFileSync at Line 36 is not guarded, so a concurrent marker deletion or filesystem error can abort this function before globalTeardown(config) runs. Likewise, an exception from restoreSeededSettings() at Line 55 skips shared cleanup. Wrap marker cleanup and restoration in error handling with globalTeardown(config) in a finally path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e-tests/global-teardown-cdp.ts` around lines 35 - 58, Update the teardown
flow around CDP_USER_DATA_FILE cleanup and restoreSeededSettings so
readFileSync, marker removal, and restoration errors cannot prevent shared
cleanup. Wrap the cleanup and restoration logic in try/catch as appropriate, and
place globalTeardown(config) in a finally path so it always executes, preserving
the existing launchedApp gating and cleanup behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e-tests/global-teardown-cdp.ts`:
- Around line 19-21: The launchedApp check in globalTeardownCdp must not infer
ownership from shared CDP_PID_FILE or CDP_USER_DATA_FILE existence. Introduce or
reuse a per-run ownership token or run-specific marker/backup established by
globalSetupCdp, and have teardown verify that token before killing the PID or
calling restoreSeededSettings(); apply the same ownership guard to the related
cleanup at the additional referenced block.

---

Outside diff comments:
In `@e2e-tests/global-teardown-cdp.ts`:
- Around line 35-58: Update the teardown flow around CDP_USER_DATA_FILE cleanup
and restoreSeededSettings so readFileSync, marker removal, and restoration
errors cannot prevent shared cleanup. Wrap the cleanup and restoration logic in
try/catch as appropriate, and place globalTeardown(config) in a finally path so
it always executes, preserving the existing launchedApp gating and cleanup
behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e7a6bd8c-e368-4407-9bda-a19dd0cf1a36

📥 Commits

Reviewing files that changed from the base of the PR and between aac46f8 and 9fd3c7c.

📒 Files selected for processing (3)

• e2e-tests/fixtures/helpers.ts
• e2e-tests/global-setup-cdp.ts
• e2e-tests/global-teardown-cdp.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• e2e-tests/global-setup-cdp.ts
• e2e-tests/fixtures/helpers.ts

[alex-rawlings-yyc]

@alex-rawlings-yyc reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on imnasnainaec).

[imnasnainaec]

Fixed CodeRabbit's "Outside diff range comments" catch.

@imnasnainaec made 3 comments and resolved 1 discussion.
Reviewable status: 7 of 8 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

---

e2e-tests/global-teardown-cdp.ts line 49 at r2 (raw file):

Previously, alex-rawlings-yyc (Alex Rawlings) wrote…

⛏️ Not ownership-gated, unlike everything else in this teardown

This runs even after a warm-instance-reuse run, which seeds nothing. Since the backup file is deliberately shared across both tiers (helpers.ts:176-180), a reuse-path teardown can consume and delete a backup belonging to a smoke run — leaving that run's seeded test values as the developer's "restored" settings.

Every other resource here is gated on the marker files; this is the one that isn't. Suggested fix:

const launchedApp = fs.existsSync(CDP_PID_FILE) || fs.existsSync(CDP_USER_DATA_FILE);

then if (launchedApp) restoreSeededSettings();

Two details that aren't obvious: the snapshot has to be taken before killProcessFromPidFile, which unlinks CDP_PID_FILE — read it after and ownership always reports false. And || rather than &&, so a run that crashed between the two marker writes still restores the seed it owns.

Low severity — the tiers run sequentially under npm run test:e2e, so this needs an interleaving to bite. Raising it because it's a one-line guard, and because the app kill and dir removal directly above both gate on the marker files. Setup's clearStaleOwnershipMarkers() comment says clearing them "keeps teardown a true no-op on this reuse path" — which this line quietly breaks.

Done

[alex-rawlings-yyc]

@alex-rawlings-yyc reviewed 1 file and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).

[alex-rawlings-yyc]

@alex-rawlings-yyc reviewed 1 file and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).