fix(e2e): overhaul all E2E specs for Linux tauri-driver

[CodeGhost21]

Summary

• Extract shared E2E helpers into app/test/e2e/helpers/shared-flows.ts (login, onboarding, navigation)
• Fix onboarding walkthrough to match real 6-step Onboarding.tsx flow (WelcomeStep → LocalAIStep → ScreenPermissionsStep → ToolsStep → SkillsStep → MnemonicStep)
• Replace all clickNativeButton() nav with window.location.hash via browser.execute() — sidebar buttons are icon-only on tauri-driver
• Use JS click as primary strategy in clickAtElement() to eliminate "element not interactable" errors
• Add error path + bypass auth tests to login-flow.spec.ts
• Fix Docker build (--no-bundle, add bash), binary path resolution, wdio.conf.ts types
• Add 5 missing specs to e2e-run-all-flows.sh
• Skip specs requiring unavailable infra on Linux CI (conversations, local-model, service-connectivity)

Test plan

• login-flow.spec.ts — 12 passing (32.9s)
• auth-access-control.spec.ts — 9 passing
• smoke.spec.ts — 3 passing
• navigation.spec.ts — 1 passing
• tauri-commands.spec.ts — 2 passing (screenshot skipped on Linux)
• screen-intelligence.spec.ts — 2 passing
• skills-registry.spec.ts — 5 passing
• card-payment-flow.spec.ts — passing
• crypto-payment-flow.spec.ts — passing
• telegram-flow.spec.ts — passing
• gmail-flow.spec.ts — passing
• notion-flow.spec.ts — passing
• conversations-web-channel-flow.spec.ts — skipped (needs streaming SSE)
• local-model-runtime.spec.ts — skipped (needs Ollama)
• service-connectivity-flow.spec.ts — skipped on Linux (gate UI auto-dismisses)

Note: This branch has merge conflicts with main due to parallel E2E changes. Conflicts are in the same files we modified and should be resolved by keeping our versions (the overhaul).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added Linux-capable end-to-end testing (alongside macOS), manual macOS trigger, Dockerized E2E environment, Xvfb/dbus lifecycle checks, broader spec coverage, shared helpers for onboarding/navigation/login, and adjusted flows/assertions for more reliable cross-platform runs.
• Chores
  • Mock API updated to support onboarding scenarios and stricter unhandled-route responses.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds Linux-first tauri-driver E2E: new GitHub Actions trigger/job, Docker image and entrypoint, Linux-specific build/run changes, a shared E2E helpers module, many spec updates to use JS/hash navigation and tauri-driver interactions, and mock-server adjustments for onboarding and unhandled routes.

Changes

Cohort / File(s)	Summary
CI / GitHub Actions \.github/workflows/test.yml	Added workflow_dispatch with run_macos_e2e input and a new e2e-linux job that builds/runs Linux E2E under Xvfb/dbus (Node 24, Rust 1.93, installs tauri-driver); macOS job commented/gated.
Docker & Entrypoint e2e/Dockerfile, e2e/docker-entrypoint.sh	New Ubuntu 22.04 E2E image installing GTK/WebKit, Rust, Node/Yarn, and tauri-driver; entrypoint starts Xvfb and dbus with startup checks, cleanup trap, and exec of the test command.
Build & Runner Scripts app/scripts/e2e-build.sh, app/scripts/e2e-run-all-flows.sh	OS-specific build: macOS builds .app bundle, Linux builds debug binary --no-bundle. Runner adds five new spec invocations to the sequential all-flows runner.
WDIO / Driver Config app/test/wdio.conf.ts	Linux app path prefers tauri target binary; capabilities typing simplified, Linux uses tauri:options.application; mocha timeout increased to 120s; exported config type broadened.
Element Helpers app/test/e2e/helpers/element-helpers.ts	clickAtElement now prefers JS e.click() via browser.execute on tauri-driver and falls back to WDIO el.click(); clickToggle uses clickAtElement for tauri-driver.
Shared Flows app/test/e2e/helpers/shared-flows.ts	New shared module exporting polling/navigation/onboarding/login utilities (e.g., waitForRequest, waitForHomePage, waitForTextToDisappear, clickFirstMatch, navigateViaHash, navigateToHome/Settings/Billing/Skills/Intelligence/Conversations, walkOnboarding, performFullLogin).
E2E Specs (many) app/test/e2e/specs/...	Numerous specs refactored to use shared-flows and JS/hash navigation (auth-access-control, card-payment-flow, conversations-web-channel-flow, crypto-payment-flow, gmail-flow, local-model-runtime, login-flow, notion-flow, service-connectivity-flow, skills-registry, tauri-commands, telegram-flow). Added platform skips/gating and self-contained mock seeding.
Service Connectivity Gating app/test/e2e/specs/service-connectivity-flow.spec.ts	Suite now skips when OPENHUMAN_SERVICE_MOCK !== '1' OR process.platform === 'linux', excluding service lifecycle tests on Linux CI.
Tauri Driver Screenshot Handling app/test/e2e/specs/tauri-commands.spec.ts	Under tauri-driver, skip screenshot byte-length assertion and assert browser.getWindowHandle() instead.
Mock API scripts/mock-api-core.mjs	Added POST /settings/onboarding-complete; catch-all now logs UNHANDLED <method> <url> and returns 404 with an error payload instead of returning 200 success.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant CI as GitHub Actions
    participant Docker as E2E Docker (Ubuntu)
    participant Xvfb as Xvfb+dbus
    participant Driver as tauri-driver
    participant App as OpenHuman (debug binary)
    participant Tests as WDIO tests (shared-flows/specs)

    CI->>Docker: build image & start container
    Docker->>Xvfb: start Xvfb/dbus (DISPLAY)
    Docker->>Driver: ensure tauri-driver installed & available
    Docker->>App: build Linux debug binary (no-bundle)
    Driver->>App: spawn/connect to app via application path
    Tests->>Driver: drive UI (DOM JS clicks, hash nav)
    Tests->>App: trigger deep-link / auth flows
    Tests->>Docker: emit logs & results
    Docker-->>CI: exit status / logs

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

• feat(e2e): move CI to Linux by default, keep macOS optional #141 — Similar Linux/tauri-driver E2E migration touching CI, Docker, WDIO, helpers, and specs.
• fix: onboarding flow - registry skills, config-based flag, dead code cleanup #177 — Related onboarding API/tauri command changes that pair with the new mock POST /settings/onboarding-complete.

🐰 I hopped into a Linux den so wide,

With Xvfb and dbus by my side,
I clicked with JS and followed hashes true,
Onboarding hopped along — the tests leapt too,
CI carrots crunchy, E2E dreams in view.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.90% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: overhauling E2E specs to support Linux tauri-driver testing infrastructure.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 18

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

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

⚠️ Outside diff range comments (1)

app/test/e2e/specs/screen-intelligence.spec.ts (1)

2-13: ⚠️ Potential issue | 🟡 Minor

Doc comment describes tests that don't exist in the file.

The doc comment lists verifications for "Settings navigation" and "Intelligence page loads without errors", but the actual tests only check app launch and chrome visibility. Either update the comment to match reality or add the missing tests.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/screen-intelligence.spec.ts` around lines 2 - 13, The
top-of-file doc comment in screen-intelligence.spec.ts claims it verifies
settings navigation and the Intelligence page, but the actual spec only checks
app launch and chrome visibility; either update the doc comment to match the
current tests or implement the missing tests. To fix: edit the doc comment block
at the top of screen-intelligence.spec.ts to remove or reword the bullet points
about "Settings navigation" and "Intelligence page loads without errors" so they
reflect only the actual checks performed (app launch and chrome visibility), or
add E2E tests (new it/describe blocks) that exercise the settings navigation
(open settings, assert screen intelligence panel elements) and the Intelligence
page load (navigate to Intelligence page, assert key elements and absence of
errors) using existing helper functions in the spec. Ensure you update or add
relevant test names so the file comment and test suite remain consistent.

🟡 Minor comments (6)

e2e/docker-compose.yml-16-29 (1)

16-29: ⚠️ Potential issue | 🟡 Minor

Comment mentions node_modules caching but volume isn't defined.

Line 18 states "node_modules/ are cached via named volumes" but only cargo registry/git volumes are defined. This is technically fine (node_modules will be in the bind mount), but the comment is misleading.

📝 Suggested comment fix

 # Notes:
 #   - The repo is bind-mounted at /app so builds persist between runs
-#   - Rust target/ and node_modules/ are cached via named volumes
+#   - Rust cargo registry/git are cached via named volumes
+#   - target/ and node_modules/ persist in the bind-mounted repo
 #   - Xvfb provides a virtual display for webkit2gtk rendering

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@e2e/docker-compose.yml` around lines 16 - 29, The top-of-file comment claims
"node_modules/ are cached via named volumes" but the e2e service's volumes list
only defines e2e-cargo-registry and e2e-cargo-git (see the e2e service and its
volumes: the bind mount ..:/app is used instead), so either add a named volume
for node_modules and reference it in the e2e service volumes or update the
comment to remove the claim; modify the docker-compose.yml comment or the e2e
service volumes (referencing the volumes section under service name "e2e") so
the comment accurately reflects the actual volumes configuration.

.github/workflows/test.yml-197-204 (1)

197-204: ⚠️ Potential issue | 🟡 Minor

Quote $SPEC_NAME to prevent word splitting.

Static analysis flagged unquoted variable expansion. While basename output typically won't contain spaces, quoting is safer and silences the warning.

🛡️ Proposed fix

             SPEC_NAME=$(basename "$spec" .spec.ts)
-            echo "=== Running $SPEC_NAME ==="
-            bash scripts/e2e-run-spec.sh "$spec" "$SPEC_NAME" || {
-              echo "FAILED: $SPEC_NAME"
-              cat /tmp/tauri-driver-e2e-${SPEC_NAME}.log 2>/dev/null || true
+            echo "=== Running ${SPEC_NAME} ==="
+            bash scripts/e2e-run-spec.sh "$spec" "${SPEC_NAME}" || {
+              echo "FAILED: ${SPEC_NAME}"
+              cat "/tmp/tauri-driver-e2e-${SPEC_NAME}.log" 2>/dev/null || true
               FAILED=1
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/test.yml around lines 197 - 204, The unquoted variable
expansion of SPEC_NAME in the workflow can cause word-splitting; change the
assignment to quote the basename call so SPEC_NAME is set using
SPEC_NAME="$(basename "$spec" .spec.ts)" and ensure all usages (e.g., echo "===
Running $SPEC_NAME ===", bash scripts/e2e-run-spec.sh "$spec" "$SPEC_NAME", cat
/tmp/tauri-driver-e2e-${SPEC_NAME}.log) remain safe—update the SPEC_NAME
assignment to use the quoted form to silence the static analysis warning and
prevent splitting.

app/test/e2e/specs/gmail-flow.spec.ts-38-39 (1)

38-39: ⚠️ Potential issue | 🟡 Minor

Local waitForHomePage shadows the imported function from shared-flows.

Same issue as in notion-flow.spec.ts—the local definition on lines 84-102 shadows the import on line 38, creating potential confusion.

🔧 Proposed fix

 import {
   performFullLogin,
   navigateToHome,
   navigateToIntelligence,
   navigateToSettings,
   waitForHomePage,
 } from '../helpers/shared-flows';

-/**
- * Wait until one of the candidate texts appears on screen (Home page markers).
- */
-async function waitForHomePage(timeout = 15_000) {
-  const candidates = [
-    'Test',
-    'Good morning',
-    'Good afternoon',
-    'Good evening',
-    'Message OpenHuman',
-    'Upgrade to Premium',
-  ];
-
-  const deadline = Date.now() + timeout;
-  while (Date.now() < deadline) {
-    for (const text of candidates) {
-      if (await textExists(text)) return text;
-    }
-    await browser.pause(1_000);
-  }
-  return null;
-}

Also applies to: 84-102

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/gmail-flow.spec.ts` around lines 38 - 39, The file defines
a local function named waitForHomePage that shadows the imported waitForHomePage
from shared-flows; locate the local definition of waitForHomePage in this spec
and either remove it so the spec uses the imported waitForHomePage, or rename
the local function (and update all local calls) to a distinct name to avoid
shadowing; ensure the top import remains and run tests to confirm the spec now
calls the intended shared-flows waitForHomePage.

app/test/e2e/specs/notion-flow.spec.ts-37-38 (1)

37-38: ⚠️ Potential issue | 🟡 Minor

Local waitForHomePage shadows the imported function from shared-flows.

Lines 83-101 define a local waitForHomePage that shadows the import on line 37. This creates confusion and potential inconsistency—callers may expect the shared version's behavior but get the local one.

Remove the local definition and use only the imported waitForHomePage from shared-flows.ts.

🔧 Proposed fix

 import {
   performFullLogin,
   navigateToHome,
   navigateToSkills,
   navigateToIntelligence,
   navigateToSettings,
   waitForHomePage,
 } from '../helpers/shared-flows';

-/**
- * Wait until one of the candidate texts appears on screen (Home page markers).
- */
-async function waitForHomePage(timeout = 15_000) {
-  const candidates = [
-    'Test',
-    'Good morning',
-    'Good afternoon',
-    'Good evening',
-    'Message OpenHuman',
-    'Upgrade to Premium',
-  ];
-
-  const deadline = Date.now() + timeout;
-  while (Date.now() < deadline) {
-    for (const text of candidates) {
-      if (await textExists(text)) return text;
-    }
-    await browser.pause(1_000);
-  }
-  return null;
-}

Also applies to: 83-101

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/notion-flow.spec.ts` around lines 37 - 38, The file
defines a local waitForHomePage that shadows the imported waitForHomePage from
shared-flows; remove the local function definition (the block named
waitForHomePage around the local helper) and update any internal references to
call the imported waitForHomePage from shared-flows instead, ensuring the import
at the top remains and no duplicate helper names exist; if the local version had
any test-specific behavior needed, move that behavior into the shared-flows
implementation or create a clearly named local helper (e.g.,
waitForHomePageLocal) and update callers accordingly.

app/test/e2e/specs/auth-access-control.spec.ts-221-223 (1)

221-223: ⚠️ Potential issue | 🟡 Minor

Use Setup later in the onboarding visibility gate.

LocalAIStep uses Setup later, not Set up later, in app/src/pages/onboarding/steps/LocalAIStep.tsx:30-55. With the current text this guard can miss the first Local AI screen and skip onboarding while the modal is still blocking the app.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 221 - 223, The
visibility gate uses the wrong onboarding text so it can miss the Local AI
modal; update the condition that builds skipVisible (which uses textExists) to
check for 'Setup later' (the exact string used by LocalAIStep) instead of 'Set
up later' so the Local AI screen is detected and onboarding isn't skipped;
reference the skipVisible variable and the textExists calls when making this
change.

app/test/e2e/specs/login-flow.spec.ts-219-223 (1)

219-223: ⚠️ Potential issue | 🟡 Minor

Use the real Setup later label in these onboarding checks.

LocalAIStep renders Setup later (no space) in app/src/pages/onboarding/steps/LocalAIStep.tsx:30-55. With Set up later here, phase 1 of Local AI setup isn't recognized, so the spec can misclassify onboarding as not visible and skip the walkthrough.

Also applies to: 243-245

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 219 - 223, The onboarding
check uses the wrong label string: replace "Set up later" with the exact label
"Setup later" wherever it appears in the test (e.g., the onboardingCandidates
array and the second occurrence later in the spec) so it matches the rendered
text in LocalAIStep (LocalAIStep component in
app/src/pages/onboarding/steps/LocalAIStep.tsx) and ensures the onboarding phase
is correctly detected; update all occurrences in the file to the exact "Setup
later" spelling.

🧹 Nitpick comments (10)

app/test/e2e/helpers/platform.ts (1)

48-57: Consider documenting the inverse relationship with isMac2().

supportsExecuteScript() currently just delegates to isTauriDriver(). A brief inline comment noting why Mac2 doesn't support execute script (only macos:* extension commands) would help future maintainers, though the doc comment above already explains this well.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/platform.ts` around lines 48 - 57, Update
supportsExecuteScript() to mention the inverse relationship with isMac2() in a
short inline comment: clarify that supportsExecuteScript() currently returns
isTauriDriver() and therefore is false for Mac2 drivers because Mac2 only
supports macos:* extension commands (reference supportsExecuteScript,
isTauriDriver, and isMac2). Add a one-line comment inside the function body
explaining this so future maintainers immediately see why Mac2 is excluded.

app/scripts/e2e-build.sh (1)

19-37: Duplicate VITE_BACKEND_URL export.

VITE_BACKEND_URL is exported on line 19 and again on line 37 with the same value. The first export (line 19) appears before the conditional .env load, while the second (line 37) appears after. If the intent is to ensure the env var is set regardless of .env contents, only the second export is needed.

🧹 Proposed cleanup

-export VITE_BACKEND_URL="http://127.0.0.1:${E2E_MOCK_PORT:-18473}"
-
 echo "Building E2E app with VITE_BACKEND_URL=$VITE_BACKEND_URL"

Move the echo after line 37 where the variable is definitively set, or remove line 19 entirely.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/scripts/e2e-build.sh` around lines 19 - 37, Remove the duplicate export
of VITE_BACKEND_URL: keep a single export after the optional dotenv load so the
final value can reflect .env overrides; specifically remove the earlier export
statement that sets VITE_BACKEND_URL using E2E_MOCK_PORT (the export near the
top) and ensure the remaining export (the one after sourcing
REPO_ROOT/scripts/load-dotenv.sh) is the definitive assignment, then move the
echo "Building E2E app..." to follow that definitive export so it logs the final
URL.

app/test/e2e/specs/screen-intelligence.spec.ts (1)

28-35: Consider adding failure diagnostics and more substantial assertions.

The tests are minimal and lack failure diagnostics. For a "Screen Intelligence" spec, consider:

1. Adding dumpAccessibilityTree() in an afterEach hook for failed tests
2. Adding actual screen intelligence UI assertions if the feature is available on Linux

Based on learnings: "Add failure diagnostics (request logs, dumpAccessibilityTree()) to E2E specs for faster debugging".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/screen-intelligence.spec.ts` around lines 28 - 35, Add
failure diagnostics and stronger assertions to the Screen Intelligence E2E spec:
add an afterEach hook that checks the test result and, on failure, calls
dumpAccessibilityTree() and collects request logs; enhance the 'app launches
with elements' and 'app chrome is visible' tests (and/or create new tests) to
assert screen-intelligence-specific UI elements or accessibility roles (use
selectors or role checks relevant to the feature) and guard Linux-only behavior
with a platform check; reference hasAppChrome(), dumpAccessibilityTree(), and
the existing it() test names when implementing these changes.

e2e/Dockerfile (1)

14-25: Consider adding --no-install-recommends to reduce image size.

Static analysis flagged missing --no-install-recommends. While the current approach ensures all transitive dependencies are installed (reducing potential runtime issues), adding this flag can significantly reduce image size.

📦 Optional image size optimization

-RUN apt-get update && apt-get install -y \
+RUN apt-get update && apt-get install -y --no-install-recommends \
   bash curl build-essential pkg-config \

Test the E2E suite after this change to ensure no required packages are missing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@e2e/Dockerfile` around lines 14 - 25, Update the Dockerfile's package
installation command (the RUN apt-get update && apt-get install -y ... line) to
include --no-install-recommends so apt-get installs only required packages and
reduces image size; keep the same package list (bash curl build-essential
pkg-config libgtk-3-dev libwebkit2gtk-4.1-dev libappindicator3-dev librsvg2-dev
patchelf libssl-dev xvfb at-spi2-core dbus-x11 webkit2gtk-driver git
ca-certificates) and retain the rm -rf /var/lib/apt/lists/* cleanup, then run
the E2E tests to verify no runtime packages are missing.

app/test/e2e/specs/smoke.spec.ts (1)

17-25: Consider adding failure diagnostics for easier debugging.

Per coding guidelines, E2E specs should include failure diagnostics (request logs, dumpAccessibilityTree()) for faster debugging. Currently, if hasAppChrome() or the element check fails, there's no diagnostic output.

🔍 Example diagnostic enhancement

+import { dumpAccessibilityTree } from '../helpers/element-helpers';
+
 describe('Smoke tests', () => {
+  afterEach(async function () {
+    if (this.currentTest?.state === 'failed') {
+      const tree = await dumpAccessibilityTree();
+      console.log('[Smoke] Failure diagnostic tree:\n', tree?.slice(0, 3000));
+    }
+  });

Based on learnings: "Add failure diagnostics (request logs, dumpAccessibilityTree()) to E2E specs for faster debugging by agents and developers".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/smoke.spec.ts` around lines 17 - 25, Wrap the two failing
assertions so that on failure they emit diagnostics: call
dumpAccessibilityTree() and capture request/network logs before rethrowing the
error; specifically, for the hasAppChrome() check and the browser.$$
element-count check, catch assertion failures (or add an afterEach that checks
test state) and invoke dumpAccessibilityTree() and your request log collector,
log their outputs with clear labels, then rethrow to preserve test failure;
reference hasAppChrome(), browser.$$, and dumpAccessibilityTree() to locate
where to add the diagnostic calls.

app/scripts/e2e-run-spec.sh (1)

21-21: Unused variable REPO_ROOT.

Static analysis correctly identifies that REPO_ROOT is defined but never used in this script.

🧹 Proposed fix

 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 APP_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
-REPO_ROOT="$(cd "$APP_DIR/.." && pwd)"
 cd "$APP_DIR"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/scripts/e2e-run-spec.sh` at line 21, The variable REPO_ROOT is defined
but never used; remove the unused assignment REPO_ROOT="$(cd "$APP_DIR/.." &&
pwd)" from the script (or, if REPO_ROOT was intended to be used, replace its
usage where needed). Locate the line that sets REPO_ROOT in e2e-run-spec.sh and
either delete that line to eliminate the unused variable or refactor code to
reference REPO_ROOT instead of APP_DIR/relative paths consistently.

app/test/e2e/specs/notion-flow.spec.ts (1)

815-826: Broadened auth verification is reasonable but could benefit from a comment.

The check now accepts any of /telegram/me, /teams, /settings, or /telegram/login-tokens/ as evidence of auth activity. This is a sensible approach for cross-platform compatibility, but a brief inline comment explaining why multiple endpoints qualify would aid future maintainers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/notion-flow.spec.ts` around lines 815 - 826, Add a short
inline comment above the auth verification block (where getRequestLog() is
called and authCall is computed) explaining that multiple endpoints
(/telegram/me, /teams, /settings, /telegram/login-tokens/) are treated as
evidence of authentication because different platforms/components hit different
endpoints during re-auth, so the test accepts any of them as confirmation of
auth activity; update the comment near the authCall variable to clearly state
this intent for future maintainers.

app/test/e2e/helpers/deep-link-helpers.ts (1)

142-175: macOS-specific commands execute unnecessarily on Linux.

When trySimulateDeepLinkInWebView fails on Linux (which it shouldn't, but defensively), the code still attempts macos: launchApp and macos: deepLink (lines 149-174) before falling through to xdg-open. These commands will always fail on Linux, adding ~750ms+ of wasted time per deep link.

Consider guarding these with a platform check.

⚡ Proposed optimization

   if (typeof browser !== 'undefined') {
     // Strategy 1: WebView simulate (works on tauri-driver, skipped on Mac2)
     if (await trySimulateDeepLinkInWebView(url)) {
       deepLinkDebug('deep link delivered via WebView simulate');
       return;
     }

+    // Strategy 2: Appium Mac2 extension commands (macOS only)
+    if (process.platform === 'darwin') {
       try {
         await browser.execute('macos: launchApp', {
           bundleId: 'com.openhuman.app',
           arguments: [url],
         } as Record<string, unknown>);
         deepLinkDebug('macos: launchApp OK');
       } catch (err) {
         deepLinkDebug('macos: launchApp failed', err instanceof Error ? err.message : err);
       }
       for (let attempt = 1; attempt <= 3; attempt += 1) {
         try {
           await browser.execute('macos: deepLink', { url, bundleId: 'com.openhuman.app' } as Record<
             string,
             unknown
           >);
           deepLinkDebug('macos: deepLink OK', { attempt });
           await browser.pause(300);
           return;
         } catch (err) {
           deepLinkDebug('macos: deepLink failed', {
             attempt,
             error: err instanceof Error ? err.message : err,
           });
           await browser.pause(250);
         }
       }
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/deep-link-helpers.ts` around lines 142 - 175, The
macOS-specific browser.execute calls in sendDeepLink (the block that calls
browser.execute('macos: launchApp', ...) and browser.execute('macos: deepLink',
...)) run even on Linux, wasting ~750ms; guard that block with a platform check
(e.g., only run if process.platform === 'darwin' or an existing isMac/isDarwin
helper) so after trySimulateDeepLinkInWebView fails on non-mac hosts you skip
the macos: launchApp/deepLink attempts and fall through to xdg-open; update the
conditional around those browser.execute calls accordingly and keep the existing
logging behavior when skipped.

app/test/e2e/helpers/app-helpers.ts (1)

105-117: Dual semantics of predicate parameter may confuse callers.

The elementExists function now interprets its predicate parameter as a CSS selector on tauri-driver but as an iOS predicate string on Mac2. This semantic overloading could lead to subtle bugs if callers don't realize the context switch.

The JSDoc warning on lines 102-103 helps, but consider renaming the parameter or adding runtime validation/logging to make the mode clearer during debugging.

💡 Alternative: explicit parameter naming

 /**
  * Check if any element matching the predicate exists.
  *
- * - Mac2: `predicate` is an iOS predicate string (e.g. `elementType == 56`)
- * - tauri-driver: `predicate` is a CSS selector (e.g. `button`, `#root`)
+ * `@param` selector - CSS selector (tauri-driver) or iOS predicate string (Mac2)
  *
  * For cross-platform specs, prefer the helpers in element-helpers.ts
  * (hasAppChrome, textExists, etc.) over calling this directly.
  */
-export async function elementExists(predicate: string): Promise<boolean> {
+export async function elementExists(selector: string): Promise<boolean> {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/app-helpers.ts` around lines 105 - 117, The
elementExists function currently overloads the predicate parameter (interpreting
it as a CSS selector when isTauriDriver() is true and as an iOS predicate string
otherwise), which is confusing; update elementExists to make the mode explicit
by either renaming the parameter (e.g., selectorOrPredicate) and adding a second
optional boolean or enum parameter (e.g., useCssSelector: boolean | 'auto') or,
if you prefer minimal change, add a runtime warning/log line in elementExists
that logs which interpretation is being used (use isTauriDriver() to decide) so
callers can see the mode, and update references to the function to pass the
explicit flag where appropriate; reference elementExists, isTauriDriver, and the
browser.$ calls to locate the implementation to change.

app/test/e2e/specs/telegram-flow.spec.ts (1)

239-242: Skipped test suite is properly documented.

The describe.skip with the explanatory comment clearly indicates this suite is temporarily disabled pending new tests for the unified Telegram system. This is appropriate—the code is preserved for reference while avoiding CI failures.

Consider creating a tracking issue to ensure these tests get rewritten.

Do you want me to open an issue to track rewriting the Telegram E2E tests for the unified system?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/telegram-flow.spec.ts` around lines 239 - 242, Create a
tracking issue to rewrite the disabled "Telegram Integration Flows" E2E suite
(currently marked with describe.skip) that explains why it was skipped, links to
the existing skipped test for reference, specifies acceptance criteria (new
unified Telegram E2E tests that cover previously tested flows), suggests
priority/labels (e.g., testing, e2e, tech-debt), and assigns or triages the
issue to the appropriate owner so the work is not forgotten.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/scripts/e2e-run-all-flows.sh`:
- Line 25: The new entry calling run
"test/e2e/specs/service-connectivity-flow.spec.ts" "service-connectivity" never
actually triggers the spec because service-connectivity-flow.spec.ts requires
OPENHUMAN_SERVICE_MOCK=1 but e2e-run-spec.sh only uses that variable if the
caller exported it; fix by ensuring the env var is set when invoking the spec
(either export OPENHUMAN_SERVICE_MOCK=1 in e2e-run-all-flows.sh before calling
run for "service-connectivity" or update e2e-run-spec.sh to default
OPENHUMAN_SERVICE_MOCK=1 when target === "service-connectivity") and keep the
change scoped to the unique identifiers service-connectivity-flow.spec.ts,
e2e-run-all-flows.sh, e2e-run-spec.sh, and OPENHUMAN_SERVICE_MOCK.

In `@app/test/e2e/helpers/element-helpers.ts`:
- Around line 310-333: clickToggle currently uses raw el.click() in the
isTauriDriver() branch which reintroduces the tauri-driver flake; update that
branch to use the same robust click helper clickAtElement(el) (the same used in
the Mac2 path) for each found selector, ensuring clickAtElement is
imported/available in element-helpers.ts and preserving the existing
fallback/error behavior ("Toggle element not found") when no selector exists.

In `@app/test/e2e/helpers/shared-flows.ts`:
- Around line 263-279: The performFullLogin function currently only asserts UI
navigation to Home and can falsely succeed if the app was already authenticated;
modify performFullLogin to accept an optional post-login verifier (e.g.,
requestLogGetter or postLoginVerifier callback) parameter, call that verifier
after waitForHomePage (and after the existing UI check), and throw/report a
failure if the verifier indicates the deep-link/auth side-effects did not occur;
reference the existing symbols triggerAuthDeepLink, waitForHomePage, and
dumpAccessibilityTree so you place the verifier invocation right after the
homeText check and before the final success console.log.
- Around line 173-175: The check in walkOnboarding() is using the wrong literal
"Set up later" so it can miss the LocalAIStep modal which renders "Setup later";
update the textExists checks in walkOnboarding() to use the exact string "Setup
later" (and ensure the variable/condition around onboardingVisible that
references textExists uses the corrected literal), and verify references to
LocalAIStep remain consistent so the helper correctly detects the onboarding
modal.
- Around line 105-145: navigateToBilling currently returns after the fallback
without verifying the billing markers; update navigateToBilling to perform a
final check using the same markers (textExists('Current Plan') ||
textExists('FREE') || textExists('Upgrade')) after the fallback pause and, if
still not found, throw an error so the test fails; before throwing capture
diagnostics (e.g., request logs via your existing network/log helper and call
dumpAccessibilityTree() if available) and include those diagnostics and the
currentHash in the thrown error message to aid debugging (use the existing
navigateViaHash, textExists and dumpAccessibilityTree helpers to locate where to
add this).

In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 469-471: The test currently assumes
waitForTextToDisappear('Waiting', 20_000) always succeeds; because
waitForTextToDisappear returns false on timeout, change the block that checks
hasWaiting to capture the returned boolean and assert or throw if it is false so
the test fails instead of logging success; update the code around hasWaiting and
the waitForTextToDisappear call to store the result (e.g., const disappeared =
await waitForTextToDisappear(...)) and then use an assertion or throw with a
clear message referencing the 'Waiting' spinner to fail the test on timeout.
- Around line 481-485: The test "3.3.1 — active subscription is displayed
correctly" depends on prior specs; update the spec to seed the expected mock
state (BASIC + planActive) locally and explicitly navigate to the Billing view
before asserting, rather than relying on previous tests — ensure the mock for
the /payments/stripe/currentPlan response is set within this test, call the
navigation helper that mounts the BillingPanel (or invoke the same logic used
elsewhere to open billing), verify the "Manage" portal button exists and assert
on its behavior (fail the test if "Manage" is missing) so the portal flow is
exercised deterministically; apply the same isolation and explicit navigation
fixes to the related block around lines 512-521.

In `@app/test/e2e/specs/card-payment-flow.spec.ts`:
- Around line 174-182: The test currently returns early when
textExists('Manage') is false, letting the spec pass without verifying backend
behavior; instead seed the active-plan state at the start of this spec (so the
app should render the portal management UI), remove the early return path and
make presence of 'Manage' mandatory (use textExists('Manage') as an assert and
fail the test if missing), and after exercising the UI call assert the mocked
Stripe portal invocation (update/resetMockBehavior usage to set the seeded
active-plan and then verify the mock for the portal call was invoked) while
keeping LOG_PREFIX, resetMockBehavior, navigateToHome, and textExists references
to locate the changes.
- Around line 67-69: The test is order-dependent because it assumes prior
auth/mock state (BASIC) instead of setting it itself; update the spec so each it
is runnable in isolation by explicitly initializing auth and mock state inside
the test (or in a beforeEach for this file). Specifically, modify the test that
calls performFullLogin('e2e-card-payment-token') to first call the project’s
auth/mock setup helper (e.g., setAuthMode('BASIC') or resetAuthMocks() /
initializeMocks()) or extend performFullLogin to accept and apply the required
auth mode, and ensure any mock that previously relied on earlier tests (the
BASIC auth mode) is set/cleared within this spec; also ensure browser
context/state is reset between tests (use a fresh page/context) so tests don’t
depend on earlier runs.

In `@app/test/e2e/specs/conversations-web-channel-flow.spec.ts`:
- Around line 35-38: The test suite for 'Conversations web channel flow' is
unconditionally skipped via describe.skip, which disables it on all platforms;
change this to a platform/capability gated run so only Linux CI is skipped.
Replace the direct describe.skip usage with a conditional wrapper that chooses
between describe and describe.skip based on a runtime check (e.g., inspect
process.platform or a test capability flag), and apply it to the existing suite
name 'Conversations web channel flow' so the suite runs on macOS/Appium but is
skipped only when the Linux condition (or missing SSE capability) is true.

In `@app/test/e2e/specs/crypto-payment-flow.spec.ts`:
- Around line 80-90: The test currently clicks Upgrade via clickText('Upgrade',
10_000) and only awaits waitForRequest('POST', '/payments/stripe/purchasePlan',
10_000), so it never exercises the crypto flow; either enable the "Pay with
Crypto" toggle before calling clickText (simulate the UI toggle that sets the
crypto state) and then assert the crypto-specific backend request (e.g.,
waitForRequest('POST', '/payments/coinbase/checkout' or whatever Coinbase
endpoint your app uses) and any crypto-specific UI changes, or rename this spec
to explicitly state it verifies the Stripe/second-card path and keep the current
Stripe assertion; update assertions and logs (e.g., the LOG_PREFIX message) to
match the chosen path so clickText, waitForRequest and the crypto toggle are
consistent.
- Around line 67-69: The failing e2e tests rely on state left by prior specs;
make each spec self-contained by explicitly creating and asserting its own auth
and subscription state instead of depending on performFullLogin or prior
runs—add a helper like createTestUserWithPlan or seedSubscriptionAndAuth (used
by the 'login and reach home' test and the tests around lines 130-164) that
programmatically creates the user, performs login, and provisions the BASIC plan
(or mocks the subscription API), invoke that helper in the test’s setup
(beforeEach or at test start), use unique test identifiers to avoid collisions,
and ensure any created state is cleaned up or namespaced so the spec can run in
isolation.

In `@app/test/e2e/specs/login-flow.spec.ts`:
- Around line 351-380: The test's heuristic using getRequestLog() and checks
like r.url.includes('Welcome') can't detect UI overlays, so replace that logic
by tracking a boolean (e.g., hadOnboardingWalkthrough) that is set when the
onboarding overlay/walkthrough is actually shown in the UI flow, then use that
boolean in the spec 'mock server received the onboarding-complete call (if
onboarding was walked)' to decide whether to require the POST
/settings/onboarding-complete call; update references to hadOnboarding to read
the new boolean and ensure expect(call).toBeDefined() only runs when
hadOnboardingWalkthrough is true, and log/assert both the UI outcome (overlay
seen) and the backend mock effect (call present) in getRequestLog().
- Around line 409-446: The test currently only verifies the consume endpoint was
hit (via waitForRequest) but never asserts the app actually rejected the token
UI-wise; update the 'expired token does not navigate to home' spec (and the
similar test at 448-469) to assert the rejection outcome by: after
waitForRequest('POST','/telegram/login-tokens/',...) confirm backend returned
401 in the captured call object, then assert UI shows the unauthenticated state
(e.g. use waitForAnyText to expect login/welcome UI like 'Welcome' or presence
of login button text, or assert absence of homeCandidates via waitForAnyText
returning false), and additionally assert client auth state cleared (check
localStorage/sessionStorage keys or that any auth token getter returns null).
Keep the existing calls to clearRequestLog, setMockBehavior, triggerDeepLink,
waitForRequest, waitForAnyText, and resetMockBehavior but add these explicit
assertions to fully validate rejection.
- Around line 475-533: The test currently only asserts there is no consume call
and logs UI/token checks, so it can pass with a stale session; update the
"bypass auth deep link sets token directly without consume call" test to (1)
ensure auth state is cleared at start (call/reset whatever clears persisted auth
used by browser.execute or localStorage in this spec, e.g., remove
'persist:auth' before triggerDeepLink), (2) after triggerDeepLink use
waitForAnyText (the existing waitForAnyText call) but replace the log with an
assertion that a post-login UI marker is found, and (3) replace the
browser.execute token check log with an assertion that tokenSet === true; keep
the existing check that getRequestLog() has no POST to '/telegram/login-tokens/'
(consumeCall) so the spec asserts both UI outcome and Redux/localStorage token
presence along with the mock/backend effect (no consume call).

In `@app/test/e2e/specs/skills-registry.spec.ts`:
- Around line 74-76: The test currently calls navigateToSkills() and returns
without verifying navigation succeeded; update the spec to assert the page
landed by checking a UI marker (e.g., assert the Skills page heading/button/list
is visible or the URL/hash changed) immediately after navigateToSkills() and
before logging via stepLog('Navigated to Skills via hash'); also assert any
relevant backend/mock effect (e.g., that the skills fetch request was made or
mock data rendered) and add failure diagnostics such as calling
dumpAccessibilityTree() and emitting request logs when the assertion fails to
aid debugging; reference navigateToSkills(), stepLog(), dumpAccessibilityTree(),
and your test helper that exposes request logs.

In `@e2e/docker-entrypoint.sh`:
- Around line 10-14: After launching Xvfb (the background process assigned to
XVFB_PID), immediately verify it started successfully and fail the entrypoint if
it died: after Xvfb :99 -screen ... & and XVFB_PID=$! check the process with a
liveliness test (e.g., kill -0 $XVFB_PID or ps) and if the check fails write an
error and exit non‑zero; also consider adding a short loop/retry (few checks
with sleep) to cover fast exits and add a trap to clean up XVFB_PID on script
exit so the container stops promptly if Xvfb cannot start.

In `@scripts/mock-api-core.mjs`:
- Around line 634-636: The catch-all mock route currently returns a successful
200 payload via the json(res, 200, { success: true, data: null }) call which
masks missing endpoints; update the fallback to fail fast by returning a non-200
error (e.g., 500 or 404) and a clear failure body (success: false, error/message
explaining unhandled route) and keep the existing console.log(`[MockServer]
UNHANDLED ${method} ${url}`) for context; locate the fallback in
scripts/mock-api-core.mjs where json(...) is called for unhandled requests and
replace that response so tests and UI see a failing response instead of a fake
success.

---

Outside diff comments:
In `@app/test/e2e/specs/screen-intelligence.spec.ts`:
- Around line 2-13: The top-of-file doc comment in screen-intelligence.spec.ts
claims it verifies settings navigation and the Intelligence page, but the actual
spec only checks app launch and chrome visibility; either update the doc comment
to match the current tests or implement the missing tests. To fix: edit the doc
comment block at the top of screen-intelligence.spec.ts to remove or reword the
bullet points about "Settings navigation" and "Intelligence page loads without
errors" so they reflect only the actual checks performed (app launch and chrome
visibility), or add E2E tests (new it/describe blocks) that exercise the
settings navigation (open settings, assert screen intelligence panel elements)
and the Intelligence page load (navigate to Intelligence page, assert key
elements and absence of errors) using existing helper functions in the spec.
Ensure you update or add relevant test names so the file comment and test suite
remain consistent.

---

Minor comments:
In @.github/workflows/test.yml:
- Around line 197-204: The unquoted variable expansion of SPEC_NAME in the
workflow can cause word-splitting; change the assignment to quote the basename
call so SPEC_NAME is set using SPEC_NAME="$(basename "$spec" .spec.ts)" and
ensure all usages (e.g., echo "=== Running $SPEC_NAME ===", bash
scripts/e2e-run-spec.sh "$spec" "$SPEC_NAME", cat
/tmp/tauri-driver-e2e-${SPEC_NAME}.log) remain safe—update the SPEC_NAME
assignment to use the quoted form to silence the static analysis warning and
prevent splitting.

In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 221-223: The visibility gate uses the wrong onboarding text so it
can miss the Local AI modal; update the condition that builds skipVisible (which
uses textExists) to check for 'Setup later' (the exact string used by
LocalAIStep) instead of 'Set up later' so the Local AI screen is detected and
onboarding isn't skipped; reference the skipVisible variable and the textExists
calls when making this change.

In `@app/test/e2e/specs/gmail-flow.spec.ts`:
- Around line 38-39: The file defines a local function named waitForHomePage
that shadows the imported waitForHomePage from shared-flows; locate the local
definition of waitForHomePage in this spec and either remove it so the spec uses
the imported waitForHomePage, or rename the local function (and update all local
calls) to a distinct name to avoid shadowing; ensure the top import remains and
run tests to confirm the spec now calls the intended shared-flows
waitForHomePage.

In `@app/test/e2e/specs/login-flow.spec.ts`:
- Around line 219-223: The onboarding check uses the wrong label string: replace
"Set up later" with the exact label "Setup later" wherever it appears in the
test (e.g., the onboardingCandidates array and the second occurrence later in
the spec) so it matches the rendered text in LocalAIStep (LocalAIStep component
in app/src/pages/onboarding/steps/LocalAIStep.tsx) and ensures the onboarding
phase is correctly detected; update all occurrences in the file to the exact
"Setup later" spelling.

In `@app/test/e2e/specs/notion-flow.spec.ts`:
- Around line 37-38: The file defines a local waitForHomePage that shadows the
imported waitForHomePage from shared-flows; remove the local function definition
(the block named waitForHomePage around the local helper) and update any
internal references to call the imported waitForHomePage from shared-flows
instead, ensuring the import at the top remains and no duplicate helper names
exist; if the local version had any test-specific behavior needed, move that
behavior into the shared-flows implementation or create a clearly named local
helper (e.g., waitForHomePageLocal) and update callers accordingly.

In `@e2e/docker-compose.yml`:
- Around line 16-29: The top-of-file comment claims "node_modules/ are cached
via named volumes" but the e2e service's volumes list only defines
e2e-cargo-registry and e2e-cargo-git (see the e2e service and its volumes: the
bind mount ..:/app is used instead), so either add a named volume for
node_modules and reference it in the e2e service volumes or update the comment
to remove the claim; modify the docker-compose.yml comment or the e2e service
volumes (referencing the volumes section under service name "e2e") so the
comment accurately reflects the actual volumes configuration.

---

Nitpick comments:
In `@app/scripts/e2e-build.sh`:
- Around line 19-37: Remove the duplicate export of VITE_BACKEND_URL: keep a
single export after the optional dotenv load so the final value can reflect .env
overrides; specifically remove the earlier export statement that sets
VITE_BACKEND_URL using E2E_MOCK_PORT (the export near the top) and ensure the
remaining export (the one after sourcing REPO_ROOT/scripts/load-dotenv.sh) is
the definitive assignment, then move the echo "Building E2E app..." to follow
that definitive export so it logs the final URL.

In `@app/scripts/e2e-run-spec.sh`:
- Line 21: The variable REPO_ROOT is defined but never used; remove the unused
assignment REPO_ROOT="$(cd "$APP_DIR/.." && pwd)" from the script (or, if
REPO_ROOT was intended to be used, replace its usage where needed). Locate the
line that sets REPO_ROOT in e2e-run-spec.sh and either delete that line to
eliminate the unused variable or refactor code to reference REPO_ROOT instead of
APP_DIR/relative paths consistently.

In `@app/test/e2e/helpers/app-helpers.ts`:
- Around line 105-117: The elementExists function currently overloads the
predicate parameter (interpreting it as a CSS selector when isTauriDriver() is
true and as an iOS predicate string otherwise), which is confusing; update
elementExists to make the mode explicit by either renaming the parameter (e.g.,
selectorOrPredicate) and adding a second optional boolean or enum parameter
(e.g., useCssSelector: boolean | 'auto') or, if you prefer minimal change, add a
runtime warning/log line in elementExists that logs which interpretation is
being used (use isTauriDriver() to decide) so callers can see the mode, and
update references to the function to pass the explicit flag where appropriate;
reference elementExists, isTauriDriver, and the browser.$ calls to locate the
implementation to change.

In `@app/test/e2e/helpers/deep-link-helpers.ts`:
- Around line 142-175: The macOS-specific browser.execute calls in sendDeepLink
(the block that calls browser.execute('macos: launchApp', ...) and
browser.execute('macos: deepLink', ...)) run even on Linux, wasting ~750ms;
guard that block with a platform check (e.g., only run if process.platform ===
'darwin' or an existing isMac/isDarwin helper) so after
trySimulateDeepLinkInWebView fails on non-mac hosts you skip the macos:
launchApp/deepLink attempts and fall through to xdg-open; update the conditional
around those browser.execute calls accordingly and keep the existing logging
behavior when skipped.

In `@app/test/e2e/helpers/platform.ts`:
- Around line 48-57: Update supportsExecuteScript() to mention the inverse
relationship with isMac2() in a short inline comment: clarify that
supportsExecuteScript() currently returns isTauriDriver() and therefore is false
for Mac2 drivers because Mac2 only supports macos:* extension commands
(reference supportsExecuteScript, isTauriDriver, and isMac2). Add a one-line
comment inside the function body explaining this so future maintainers
immediately see why Mac2 is excluded.

In `@app/test/e2e/specs/notion-flow.spec.ts`:
- Around line 815-826: Add a short inline comment above the auth verification
block (where getRequestLog() is called and authCall is computed) explaining that
multiple endpoints (/telegram/me, /teams, /settings, /telegram/login-tokens/)
are treated as evidence of authentication because different platforms/components
hit different endpoints during re-auth, so the test accepts any of them as
confirmation of auth activity; update the comment near the authCall variable to
clearly state this intent for future maintainers.

In `@app/test/e2e/specs/screen-intelligence.spec.ts`:
- Around line 28-35: Add failure diagnostics and stronger assertions to the
Screen Intelligence E2E spec: add an afterEach hook that checks the test result
and, on failure, calls dumpAccessibilityTree() and collects request logs;
enhance the 'app launches with elements' and 'app chrome is visible' tests
(and/or create new tests) to assert screen-intelligence-specific UI elements or
accessibility roles (use selectors or role checks relevant to the feature) and
guard Linux-only behavior with a platform check; reference hasAppChrome(),
dumpAccessibilityTree(), and the existing it() test names when implementing
these changes.

In `@app/test/e2e/specs/smoke.spec.ts`:
- Around line 17-25: Wrap the two failing assertions so that on failure they
emit diagnostics: call dumpAccessibilityTree() and capture request/network logs
before rethrowing the error; specifically, for the hasAppChrome() check and the
browser.$$ element-count check, catch assertion failures (or add an afterEach
that checks test state) and invoke dumpAccessibilityTree() and your request log
collector, log their outputs with clear labels, then rethrow to preserve test
failure; reference hasAppChrome(), browser.$$, and dumpAccessibilityTree() to
locate where to add the diagnostic calls.

In `@app/test/e2e/specs/telegram-flow.spec.ts`:
- Around line 239-242: Create a tracking issue to rewrite the disabled "Telegram
Integration Flows" E2E suite (currently marked with describe.skip) that explains
why it was skipped, links to the existing skipped test for reference, specifies
acceptance criteria (new unified Telegram E2E tests that cover previously tested
flows), suggests priority/labels (e.g., testing, e2e, tech-debt), and assigns or
triages the issue to the appropriate owner so the work is not forgotten.

In `@e2e/Dockerfile`:
- Around line 14-25: Update the Dockerfile's package installation command (the
RUN apt-get update && apt-get install -y ... line) to include
--no-install-recommends so apt-get installs only required packages and reduces
image size; keep the same package list (bash curl build-essential pkg-config
libgtk-3-dev libwebkit2gtk-4.1-dev libappindicator3-dev librsvg2-dev patchelf
libssl-dev xvfb at-spi2-core dbus-x11 webkit2gtk-driver git ca-certificates) and
retain the rm -rf /var/lib/apt/lists/* cleanup, then run the E2E tests to verify
no runtime packages are missing.

🪄 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

Run ID: 72105b41-a72b-4e49-8766-dd6aaa40c4fc

📥 Commits

Reviewing files that changed from the base of the PR and between 7ce9011 and 3201ffb.

📒 Files selected for processing (32)

• .github/workflows/test.yml
• CLAUDE.md
• app/scripts/e2e-build.sh
• app/scripts/e2e-run-all-flows.sh
• app/scripts/e2e-run-spec.sh
• app/src-tauri/src/lib.rs
• app/test/e2e/helpers/app-helpers.ts
• app/test/e2e/helpers/deep-link-helpers.ts
• app/test/e2e/helpers/element-helpers.ts
• app/test/e2e/helpers/platform.ts
• app/test/e2e/helpers/shared-flows.ts
• app/test/e2e/specs/auth-access-control.spec.ts
• app/test/e2e/specs/card-payment-flow.spec.ts
• app/test/e2e/specs/conversations-web-channel-flow.spec.ts
• app/test/e2e/specs/crypto-payment-flow.spec.ts
• app/test/e2e/specs/gmail-flow.spec.ts
• app/test/e2e/specs/local-model-runtime.spec.ts
• app/test/e2e/specs/login-flow.spec.ts
• app/test/e2e/specs/navigation.spec.ts
• app/test/e2e/specs/notion-flow.spec.ts
• app/test/e2e/specs/screen-intelligence.spec.ts
• app/test/e2e/specs/service-connectivity-flow.spec.ts
• app/test/e2e/specs/skills-registry.spec.ts
• app/test/e2e/specs/smoke.spec.ts
• app/test/e2e/specs/tauri-commands.spec.ts
• app/test/e2e/specs/telegram-flow.spec.ts
• app/test/wdio.conf.ts
• docs/E2E-TESTING.md
• e2e/Dockerfile
• e2e/docker-compose.yml
• e2e/docker-entrypoint.sh
• scripts/mock-api-core.mjs

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use the actual Setup later copy in walkOnboarding().

LocalAIStep renders Setup later (no space). With Set up later here, the helper can skip onboarding when the app resumes on the first Local AI step, even though the modal is still active.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/shared-flows.ts` around lines 173 - 175, The check in
walkOnboarding() is using the wrong literal "Set up later" so it can miss the
LocalAIStep modal which renders "Setup later"; update the textExists checks in
walkOnboarding() to use the exact string "Setup later" (and ensure the
variable/condition around onboardingVisible that references textExists uses the
corrected literal), and verify references to LocalAIStep remain consistent so
the helper correctly detects the onboarding modal.

[coderabbitai]

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (1)

app/test/e2e/specs/notion-flow.spec.ts (1)

149-157: ⚠️ Potential issue | 🟠 Major

Navigation failures can be silently ignored, causing false passes

navigateToIntelligence() currently uses hash navigation that catches errors internally, so these try/catch blocks won’t reliably detect failures. Add an explicit post-navigation assertion (hash or page marker) right after navigation to prevent silent pass paths.

Proposed hardening

   try {
     await navigateToIntelligence();
+    const hash = await browser.execute(() => window.location.hash);
+    expect(hash).toContain('/intelligence');
     if (await textExists('Notion')) {
       console.log(`${LOG_PREFIX} Notion found on Intelligence page`);
       return true;
     }
   } catch {

       try {
         await navigateToIntelligence();
+        const hash = await browser.execute(() => window.location.hash);
+        expect(hash).toContain('/intelligence');
         await browser.pause(3_000);
         console.log(`${LOG_PREFIX} 8.2.1: Navigated to Intelligence page`);
       } catch {

As per coding guidelines "Assert both UI outcomes and backend/mock effects in E2E specs when relevant to ensure full-stack correctness".

Also applies to: 430-437

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/notion-flow.spec.ts` around lines 149 - 157, The try/catch
around navigateToIntelligence() is insufficient because navigateToIntelligence()
swallows navigation errors; after calling navigateToIntelligence() (and before
checking textExists('Notion')), add an explicit post-navigation assertion such
as verifying the location hash or a known page DOM marker (e.g., assert
window.location.hash contains the intelligence route or assert a specific
header/id is present) to fail fast on navigation errors; update the same pattern
elsewhere in the file (the other try/catch that uses
navigateToIntelligence()/textExists) so both places assert a concrete navigation
result rather than relying solely on textExists().

🧹 Nitpick comments (1)

app/test/e2e/specs/auth-access-control.spec.ts (1)

106-155: Move these Linux-specific flows into the shared E2E helper layer.

This spec now owns hash navigation, onboarding, and login orchestration inline, which is what pushed it past ~650 lines. Importing the shared flow helpers here keeps the spec focused on assertions and prevents tauri-driver workarounds from drifting across other specs.

As per coding guidelines, Keep source files at ≤ ~500 lines per file; split modules when growing larger and In E2E specs, use helpers from app/test/e2e/helpers/ (element-helpers.ts, platform.ts, deep-link-helpers.ts, app-helpers.ts).

Also applies to: 211-351

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 106 - 155, The
Linux/tauri-specific navigation and login helper functions (navigateViaHash,
clickNavByAriaLabel, navigateToHome, navigateToSettings) should be extracted
from this spec into the shared E2E helper layer (app/test/e2e/helpers/*);
replace the inline implementations with imports from the appropriate helper
module (e.g., platform or app-helpers) and update calls in this spec to use
those shared helpers so the spec only contains assertions; ensure the new helper
exports preserve behavior (hash navigation, JS aria-label click fallback, pause
and wait semantics, and the onboarding/login orchestration) and remove the
duplicated code from the spec.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 96-104: clickFirstMatch currently probes each candidate only once
which bypasses the provided timeout and can click non-action elements; update
clickFirstMatch to poll until timeout for each candidate using textExists
repeatedly (or await textExists with a wait loop) before moving to the next
candidate, and ensure it returns only after a successful click via
clickText(text, timeout). In walkOnboarding fix the candidate strings to match
the UI exactly (use "Setup later" not "Set up later") and remove "Use Local
Models" from clickable targets (it's a step title, not a button), and in the
branch that assumes the overlay is not visible add a short polling loop (e.g., a
few retries with small delays) before concluding onboarding never mounted.
Reference: clickFirstMatch, walkOnboarding, textExists, clickText.
- Around line 296-317: Detect the recovery-phrase screen by calling
textExists('Your Recovery Phrase') (used in MnemonicStep / performFullLogin)
and, when true, skip or redact any accessibility-tree dumps or console logs that
could include the phrase or other secrets; wrap the existing dump/log calls (and
the ones referenced around the same flow) with a guard that either returns a
sanitized placeholder like '[REDACTED]' or entirely skips invoking the dump
function so nothing sensitive is emitted (apply this check where accessibility
dumps are emitted after performFullLogin and in the MnemonicStep code paths that
call clickFirstMatch).
- Around line 597-611: The confirmation click routine may miss non-<button>
controls and doesn't assert it ran; update the block that calls
browser.execute() (the anonymous function inside the confirm handling) to search
for elements with role="button" and ARIA labels/text as well as <button>,
perform the click, and return a boolean indicating whether a matching control
was found and clicked; then capture that boolean in the test and assert it is
true. Also tighten the final postcondition after logout by replacing the generic
"not on Home" check with an explicit logged-out marker assertion (e.g., presence
of the login button or an element/text that signals the logged-out state) so
Settings cannot falsely satisfy the test.

In `@app/test/e2e/specs/notion-flow.spec.ts`:
- Around line 32-39: The file currently defines a local helper waitForHomePage
that duplicates and shadows the imported waitForHomePage from shared-flows;
remove the local function declaration named waitForHomePage (the entire local
helper block) so the module uses the imported waitForHomePage, keep the existing
import line in the top imports, and run the E2E tests to verify no references
were broken and the shared helper is used everywhere.

---

Outside diff comments:
In `@app/test/e2e/specs/notion-flow.spec.ts`:
- Around line 149-157: The try/catch around navigateToIntelligence() is
insufficient because navigateToIntelligence() swallows navigation errors; after
calling navigateToIntelligence() (and before checking textExists('Notion')), add
an explicit post-navigation assertion such as verifying the location hash or a
known page DOM marker (e.g., assert window.location.hash contains the
intelligence route or assert a specific header/id is present) to fail fast on
navigation errors; update the same pattern elsewhere in the file (the other
try/catch that uses navigateToIntelligence()/textExists) so both places assert a
concrete navigation result rather than relying solely on textExists().

---

Nitpick comments:
In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 106-155: The Linux/tauri-specific navigation and login helper
functions (navigateViaHash, clickNavByAriaLabel, navigateToHome,
navigateToSettings) should be extracted from this spec into the shared E2E
helper layer (app/test/e2e/helpers/*); replace the inline implementations with
imports from the appropriate helper module (e.g., platform or app-helpers) and
update calls in this spec to use those shared helpers so the spec only contains
assertions; ensure the new helper exports preserve behavior (hash navigation, JS
aria-label click fallback, pause and wait semantics, and the onboarding/login
orchestration) and remove the duplicated code from the spec.

🪄 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

Run ID: aed00f9a-14ea-44f9-89a7-52668e897483

📥 Commits

Reviewing files that changed from the base of the PR and between 3201ffb and c3a70e9.

📒 Files selected for processing (4)

• app/test/e2e/specs/auth-access-control.spec.ts
• app/test/e2e/specs/gmail-flow.spec.ts
• app/test/e2e/specs/notion-flow.spec.ts
• app/test/e2e/specs/telegram-flow.spec.ts

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

• app/test/e2e/specs/telegram-flow.spec.ts
• app/test/e2e/specs/gmail-flow.spec.ts

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

app/test/e2e/specs/login-flow.spec.ts (1)

386-395: ⚠️ Potential issue | 🟠 Major

Guard the failure dump against MnemonicStep.

If Home never loads and the app is still on Your Recovery Phrase, dumpAccessibilityTree() will emit the recovery phrase into CI logs. Redact or skip this dump whenever that screen is visible.

🔒 Suggested guard

     if (foundText) {
       console.log(`[LoginFlow] Home page confirmed: found "${foundText}"`);
     } else {
-      const tree = await dumpAccessibilityTree();
-      console.log('[LoginFlow] Home page text not found. Tree:\n', tree.slice(0, 4000));
+      if (await textExists('Your Recovery Phrase')) {
+        console.log(
+          '[LoginFlow] Home page text not found. Tree dump redacted — recovery phrase visible.'
+        );
+      } else {
+        const tree = await dumpAccessibilityTree();
+        console.log('[LoginFlow] Home page text not found. Tree:\n', tree.slice(0, 4000));
+      }
     }

As per coding guidelines "Never log secrets, raw JWTs, API keys, or full PII—redact or omit sensitive fields".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 386 - 395, The failure
dump currently calls dumpAccessibilityTree() unconditionally and can leak the
recovery phrase when the app is on the MnemonicStep; update the block that
follows waitForAnyText(foundText) to first detect whether the MnemonicStep (or
the selector/text that uniquely identifies the recovery-phrase screen) is
present/visible (reuse the same query utilities used elsewhere, e.g. a selector
or helper for MnemonicStep), and only call dumpAccessibilityTree() if that
screen is NOT visible; if you must emit a tree while MnemonicStep is visible,
redact the sensitive subtree (replace the mnemonic text with a fixed placeholder
like "[REDACTED MNEMONIC]") before logging. Ensure you change the logic around
foundText / dumpAccessibilityTree to use this guard so secrets are never
printed.

♻️ Duplicate comments (5)

app/test/e2e/specs/card-payment-flow.spec.ts (1)

99-102: ⚠️ Potential issue | 🟠 Major

These billing cases are only half self-contained.

Seeding plan and planActive fixes the subscription state, but the cases still rely on the authenticated session created in login and reach home. Retrying any of them in isolation starts from the wrong auth state before navigateToBilling() ever mounts Billing. Please move the login bootstrap into beforeEach or into each case that needs it.

Based on learnings: Ensure each E2E spec is runnable in isolation without dependencies on other specs.

Also applies to: 154-156, 176-179

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/card-payment-flow.spec.ts` around lines 99 - 102, The
tests seed plan-related mocks (setMockBehavior('plan', ...),
setMockBehavior('planActive', ...), setMockBehavior('planExpiry', ...)) but
still depend on the authenticated session established by the "login and reach
home" test, making them fail when run in isolation; move the authentication
bootstrap so each billing test gets a fresh logged-in session by calling the
existing login helper (the same logic used in the "login and reach home" spec)
either inside a beforeEach for the spec file or at the start of each test that
calls navigateToBilling(), ensuring each test is self-contained and runnable
independently from others.

app/test/e2e/specs/auth-access-control.spec.ts (2)

245-252: ⚠️ Potential issue | 🟠 Major

The post-login tree dump can still expose the recovery phrase.

walkOnboarding() redacts MnemonicStep logging, but this fallback still dumps the full page source if Home never loads. When the flow is stuck on Your Recovery Phrase, that secret lands in CI logs.

🔒 Suggested guard

   const homeText = await waitForHomePage(15_000);
   if (!homeText) {
-    const tree = await dumpAccessibilityTree();
-    console.log('[AuthAccess] Home page not reached after login. Tree:\n', tree.slice(0, 4000));
+    if (await textExists('Your Recovery Phrase')) {
+      console.log(
+        '[AuthAccess] Home page not reached after login. Tree dump redacted — recovery phrase visible.'
+      );
+    } else {
+      const tree = await dumpAccessibilityTree();
+      console.log('[AuthAccess] Home page not reached after login. Tree:\n', tree.slice(0, 4000));
+    }
     throw new Error('Full login did not reach Home page');
   }

As per coding guidelines "Never log secrets, raw JWTs, API keys, or full PII—redact or omit sensitive fields".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 245 - 252, The
fallback accessibility tree dump can leak the recovery phrase; instead of
logging the raw output from dumpAccessibilityTree() after waitForHomePage fails,
sanitize or omit secrets by calling a redaction routine (e.g.,
redactSensitiveInfo or dumpAccessibilityTree({redact: true})) before logging, or
detect known sensitive markers like "Recovery Phrase", "Your Recovery Phrase",
or 12/24-word mnemonic patterns and replace them with a fixed placeholder;
update the post-login block around walkOnboarding(), waitForHomePage(), and
dumpAccessibilityTree() (and consider referring to MnemonicStep) to use the
redacted output or skip dumping when sensitive content is present.

---

521-548: ⚠️ Potential issue | 🟠 Major

Require the logged-out screen here.

expect(onWelcome || !hasToken) still lets this pass when storage is cleared but the app stays on Settings or another stale screen. This flow should require a logged-out UI marker and keep the token-cleared check as a second assertion.

🚪 Suggested assertion

-    // Must see logged-out UI or token must be cleared (or both)
-    expect(onWelcome || !hasToken).toBe(true);
-    console.log(`[AuthAccess] Logout verified: welcomeUI=${onWelcome}, tokenCleared=${!hasToken}`);
+    expect(onWelcome).toBe(true);
+    expect(hasToken).toBe(false);
+    console.log('[AuthAccess] Logout verified: welcome UI visible and token cleared');

As per coding guidelines "Assert both UI outcomes and backend/mock effects in E2E specs when relevant to ensure full-stack correctness".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 521 - 548, The
test currently allows passing if either the logged-out UI marker (onWelcome) or
token-cleared (hasToken) is true; change it to require the logged-out UI
explicitly and keep the storage check as a separate assertion: assert onWelcome
is true (using the existing onWelcome variable and textExists checks) and then
assert that hasToken is false (the browser.execute result) so both UI and
token-clearing are verified independently.

app/test/e2e/specs/crypto-payment-flow.spec.ts (1)

71-108: ⚠️ Potential issue | 🟠 Major

This “Coinbase charge” case still passes on the Stripe path.

clickToggle() is not scoped to the “Pay with Crypto” control, and the final expectation accepts /payments/stripe/purchasePlan as success. If the wrong toggle is clicked or the crypto switch never takes effect, 6.1.1 still goes green without exercising the Coinbase path the title claims.

💳 Suggested assertion tightening

-    // Verify a payment API was called — prefer Coinbase, fall back to Stripe
+    // Verify the crypto path actually hit Coinbase
     const coinbaseCall = await waitForRequest('POST', '/payments/coinbase/charge', 10_000);
-    const stripeCall = !coinbaseCall
-      ? await waitForRequest('POST', '/payments/stripe/purchasePlan', 5_000)
-      : null;
-
-    if (coinbaseCall) {
-      console.log(`${LOG_PREFIX} 6.1.1 — Coinbase charge API called (crypto path)`);
-    } else if (stripeCall) {
-      console.log(`${LOG_PREFIX} 6.1.1 — Stripe API called (crypto toggle may not have taken effect)`);
-    }
-    expect(coinbaseCall || stripeCall).toBeDefined();
+    expect(coinbaseCall).toBeDefined();
+    console.log(`${LOG_PREFIX} 6.1.1 — Coinbase charge API called (crypto path)`);

As per coding guidelines "Assert both UI outcomes and backend/mock effects in E2E specs when relevant to ensure full-stack correctness".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/crypto-payment-flow.spec.ts` around lines 71 - 108, The
test is passing via the Stripe fallback because clickToggle() is not scoped and
the final expectation accepts Stripe; update the spec to explicitly target the
"Pay with Crypto" control and assert Coinbase was called: replace the ambiguous
await clickToggle(10_000) with a scoped action (e.g. a helper that clicks the
specific toggle element or call clickText('Pay with Crypto') before toggling, or
use a selector-based click on the toggle adjacent to the 'Pay with Crypto'
label), verify the toggle UI state changed (use textExists/getToggleState or
check the toggle element's class/aria-checked), then call Upgrade and assert
that waitForRequest('POST','/payments/coinbase/charge', 10_000) returns a truthy
value and fail the test if it does not (do not treat the Stripe call as success
for this case); keep the existing logs but tighten the final expect to require
coinbaseCall toBeDefined.

app/test/e2e/specs/skills-registry.spec.ts (1)

74-100: ⚠️ Potential issue | 🟠 Major

Also assert the skills fetch in this navigation test.

clearRequestLog() is already reset before navigateToSkills(), but this case only checks hash and text markers. A cached or partially rendered view can still satisfy those assertions without ever loading the registry data, so this scenario can pass while the page's real backend dependency is broken.

🧪 Suggested assertion

     expect(hasSkillsContent).toBe(true);
+    const skillsRequest = await waitForRequest('GET', '/skills', 10_000);
+    expect(skillsRequest).toBeDefined();
     stepLog('Skills page verified');

As per coding guidelines "Assert both UI outcomes and backend/mock effects in E2E specs when relevant to ensure full-stack correctness".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/skills-registry.spec.ts` around lines 74 - 100, After
navigating with navigateToSkills() you currently only assert hash and UI text;
also assert that the Skills registry network call was made and succeeded by
inspecting getRequestLog() for the expected fetch (e.g., a GET to the skills
registry endpoint or a request containing "skills" and a 2xx status). Use
clearRequestLog() (already called) then after navigateToSkills() read
getRequestLog() and assert the presence of the skills request and a successful
response; reference functions: clearRequestLog(), navigateToSkills(),
getRequestLog(), textExists(), dumpAccessibilityTree(), and stepLog() to locate
the relevant block to add this assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/test/e2e/specs/login-flow.spec.ts`:
- Around line 95-103: clickFirstMatch currently does a single pass over
candidates and never waits up to the provided timeout, so on slow renders it may
return null prematurely; change it to poll until the overall timeout expires:
compute a deadline (now+timeout), loop until deadline, on each iteration iterate
candidates and call textExists with a short per-check timeout (or immediate
check), if found call clickText and return the text, otherwise sleep a short
interval and retry until deadline then return null. Apply the same
polling/timeout pattern to the related helper used by the onboarding check (the
hadOnboardingWalkthrough logic referenced in the review) so both helpers respect
the supplied timeout.
- Around line 132-138: The test suite "Login flow — complete with mock data
(Linux)" currently shares state created in the top-level before hook
(startMockServer, waitForApp, clearRequestLog, hadOnboardingWalkthrough),
causing cross-test coupling; update the spec so each flow is isolated by either
(A) moving the onboarding, 401-path, and bypass cases into separate describe
blocks (each with its own before/after to call startMockServer/waitForApp and
clearRequestLog) or (B) add a beforeEach that rebuilds a clean auth/session and
UI state before every it (clear cookies/localStorage, create a fresh browser
window/context, reset hadOnboardingWalkthrough, and ensure waitForApp runs) so
every spec can run independently and deterministically.

---

Outside diff comments:
In `@app/test/e2e/specs/login-flow.spec.ts`:
- Around line 386-395: The failure dump currently calls dumpAccessibilityTree()
unconditionally and can leak the recovery phrase when the app is on the
MnemonicStep; update the block that follows waitForAnyText(foundText) to first
detect whether the MnemonicStep (or the selector/text that uniquely identifies
the recovery-phrase screen) is present/visible (reuse the same query utilities
used elsewhere, e.g. a selector or helper for MnemonicStep), and only call
dumpAccessibilityTree() if that screen is NOT visible; if you must emit a tree
while MnemonicStep is visible, redact the sensitive subtree (replace the
mnemonic text with a fixed placeholder like "[REDACTED MNEMONIC]") before
logging. Ensure you change the logic around foundText / dumpAccessibilityTree to
use this guard so secrets are never printed.

---

Duplicate comments:
In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 245-252: The fallback accessibility tree dump can leak the
recovery phrase; instead of logging the raw output from dumpAccessibilityTree()
after waitForHomePage fails, sanitize or omit secrets by calling a redaction
routine (e.g., redactSensitiveInfo or dumpAccessibilityTree({redact: true}))
before logging, or detect known sensitive markers like "Recovery Phrase", "Your
Recovery Phrase", or 12/24-word mnemonic patterns and replace them with a fixed
placeholder; update the post-login block around walkOnboarding(),
waitForHomePage(), and dumpAccessibilityTree() (and consider referring to
MnemonicStep) to use the redacted output or skip dumping when sensitive content
is present.
- Around line 521-548: The test currently allows passing if either the
logged-out UI marker (onWelcome) or token-cleared (hasToken) is true; change it
to require the logged-out UI explicitly and keep the storage check as a separate
assertion: assert onWelcome is true (using the existing onWelcome variable and
textExists checks) and then assert that hasToken is false (the browser.execute
result) so both UI and token-clearing are verified independently.

In `@app/test/e2e/specs/card-payment-flow.spec.ts`:
- Around line 99-102: The tests seed plan-related mocks (setMockBehavior('plan',
...), setMockBehavior('planActive', ...), setMockBehavior('planExpiry', ...))
but still depend on the authenticated session established by the "login and
reach home" test, making them fail when run in isolation; move the
authentication bootstrap so each billing test gets a fresh logged-in session by
calling the existing login helper (the same logic used in the "login and reach
home" spec) either inside a beforeEach for the spec file or at the start of each
test that calls navigateToBilling(), ensuring each test is self-contained and
runnable independently from others.

In `@app/test/e2e/specs/crypto-payment-flow.spec.ts`:
- Around line 71-108: The test is passing via the Stripe fallback because
clickToggle() is not scoped and the final expectation accepts Stripe; update the
spec to explicitly target the "Pay with Crypto" control and assert Coinbase was
called: replace the ambiguous await clickToggle(10_000) with a scoped action
(e.g. a helper that clicks the specific toggle element or call clickText('Pay
with Crypto') before toggling, or use a selector-based click on the toggle
adjacent to the 'Pay with Crypto' label), verify the toggle UI state changed
(use textExists/getToggleState or check the toggle element's
class/aria-checked), then call Upgrade and assert that
waitForRequest('POST','/payments/coinbase/charge', 10_000) returns a truthy
value and fail the test if it does not (do not treat the Stripe call as success
for this case); keep the existing logs but tighten the final expect to require
coinbaseCall toBeDefined.

In `@app/test/e2e/specs/skills-registry.spec.ts`:
- Around line 74-100: After navigating with navigateToSkills() you currently
only assert hash and UI text; also assert that the Skills registry network call
was made and succeeded by inspecting getRequestLog() for the expected fetch
(e.g., a GET to the skills registry endpoint or a request containing "skills"
and a 2xx status). Use clearRequestLog() (already called) then after
navigateToSkills() read getRequestLog() and assert the presence of the skills
request and a successful response; reference functions: clearRequestLog(),
navigateToSkills(), getRequestLog(), textExists(), dumpAccessibilityTree(), and
stepLog() to locate the relevant block to add this assertion.

🪄 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

Run ID: 152bef93-65ca-481d-95af-8e7892f81821

📥 Commits

Reviewing files that changed from the base of the PR and between c3a70e9 and ea23d2b.

📒 Files selected for processing (12)

• app/scripts/e2e-run-all-flows.sh
• app/test/e2e/helpers/element-helpers.ts
• app/test/e2e/helpers/shared-flows.ts
• app/test/e2e/specs/auth-access-control.spec.ts
• app/test/e2e/specs/card-payment-flow.spec.ts
• app/test/e2e/specs/conversations-web-channel-flow.spec.ts
• app/test/e2e/specs/crypto-payment-flow.spec.ts
• app/test/e2e/specs/login-flow.spec.ts
• app/test/e2e/specs/notion-flow.spec.ts
• app/test/e2e/specs/skills-registry.spec.ts
• e2e/docker-entrypoint.sh
• scripts/mock-api-core.mjs

✅ Files skipped from review due to trivial changes (2)

• e2e/docker-entrypoint.sh
• app/test/e2e/helpers/shared-flows.ts

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

• app/scripts/e2e-run-all-flows.sh
• app/test/e2e/specs/conversations-web-channel-flow.spec.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

clickFirstMatch() still ignores the timeout.

The helper makes a single pass over candidates, and the walkthrough also does a one-shot onboarding visibility check before returning. On slower Linux runs that silently skips late-rendering steps instead of waiting for them, which makes the walkthrough and hadOnboardingWalkthrough flaky.

⏱️ Suggested fix

 async function clickFirstMatch(candidates, timeout = 5_000) {
-  for (const text of candidates) {
-    if (await textExists(text)) {
-      await clickText(text, timeout);
-      return text;
-    }
-  }
+  const deadline = Date.now() + timeout;
+  while (Date.now() < deadline) {
+    for (const text of candidates) {
+      if (await textExists(text)) {
+        await clickText(text, Math.max(deadline - Date.now(), 1_000));
+        return text;
+      }
+    }
+    await browser.pause(500);
+  }
   return null;
 }

Also applies to: 246-257

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 95 - 103, clickFirstMatch
currently does a single pass over candidates and never waits up to the provided
timeout, so on slow renders it may return null prematurely; change it to poll
until the overall timeout expires: compute a deadline (now+timeout), loop until
deadline, on each iteration iterate candidates and call textExists with a short
per-check timeout (or immediate check), if found call clickText and return the
text, otherwise sleep a short interval and retry until deadline then return
null. Apply the same polling/timeout pattern to the related helper used by the
onboarding check (the hadOnboardingWalkthrough logic referenced in the review)
so both helpers respect the supplied timeout.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

This suite still shares state across phases.

The onboarding, 401-path, and bypass cases all reuse the live session/window created by earlier it blocks. That makes retries and isolated runs non-deterministic, and it weakens the negative-path checks because they can start from an already authenticated app. Please collapse each flow into its own spec or rebuild auth state in beforeEach.

Based on learnings: Ensure each E2E spec is runnable in isolation without dependencies on other specs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 132 - 138, The test suite
"Login flow — complete with mock data (Linux)" currently shares state created in
the top-level before hook (startMockServer, waitForApp, clearRequestLog,
hadOnboardingWalkthrough), causing cross-test coupling; update the spec so each
flow is isolated by either (A) moving the onboarding, 401-path, and bypass cases
into separate describe blocks (each with its own before/after to call
startMockServer/waitForApp and clearRequestLog) or (B) add a beforeEach that
rebuilds a clean auth/session and UI state before every it (clear
cookies/localStorage, create a fresh browser window/context, reset
hadOnboardingWalkthrough, and ensure waitForApp runs) so every spec can run
independently and deterministically.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (7)

app/test/e2e/helpers/shared-flows.ts (2)

305-311: ⚠️ Potential issue | 🟠 Major

Redact the failure dump when login may still be on MnemonicStep.

If walkOnboarding() stalls on Your Recovery Phrase, this log prints the recovery phrase into test output. Guard the dump with a recovery-phrase check or replace it with a redacted placeholder. As per coding guidelines "Never log secrets, raw JWTs, API keys, or full PII—redact or omit sensitive fields".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/shared-flows.ts` around lines 305 - 311, The failure
dump printed after walkOnboarding/waitForHomePage can leak the recovery phrase
when the flow is stuck on MnemonicStep; modify the error handling in the block
that calls waitForHomePage and dumpAccessibilityTree so that before logging or
including the accessibility tree you detect if the current screen/component is
MnemonicStep (or contains labels like "Your Recovery Phrase"/"recovery phrase")
and, if so, either replace sensitive fields in the dumped tree with a redacted
placeholder (e.g., "[REDACTED RECOVERY PHRASE]") or omit the tree entirely, then
log the safe/redacted message and throw the error — update the code around
walkOnboarding, waitForHomePage, and dumpAccessibilityTree accordingly.

---

64-71: ⚠️ Potential issue | 🟠 Major

clickFirstMatch() still ignores its timeout, and walkOnboarding() still gives up after one probe.

On slower tauri-driver renders this can return null/skip onboarding before the next step mounts, so callers falsely treat the overlay as absent.

Also applies to: 197-210

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/shared-flows.ts` around lines 64 - 71, clickFirstMatch
ignores its timeout and can return null too quickly; change it so each candidate
uses the provided timeout when waiting for presence (call textExists(text,
timeout) instead of textExists(text)) and pass the same timeout into clickText;
implement waiting/polling up to that timeout per candidate rather than a single
instantaneous probe. Also update walkOnboarding to call clickFirstMatch with an
appropriate per-step timeout (or loop/retry until an overall timeout) so it
doesn't give up after one probe; reference the clickFirstMatch, textExists,
clickText, and walkOnboarding symbols when making these changes.

app/test/e2e/specs/auth-access-control.spec.ts (2)

517-544: ⚠️ Potential issue | 🟠 Major

Logout can still pass without reaching logged-out UI.

expect(onWelcome || !hasToken) goes green when storage is cleared but the app stays on Settings. This spec should require the visible logged-out marker and the cleared token so the user-facing outcome is actually proven.

🔐 Tighten the postcondition

-    expect(onWelcome || !hasToken).toBe(true);
-    console.log(`[AuthAccess] Logout verified: welcomeUI=${onWelcome}, tokenCleared=${!hasToken}`);
+    expect(onWelcome).toBe(true);
+    expect(hasToken).toBe(false);
+    console.log('[AuthAccess] Logout verified: welcome UI visible and token cleared');

Based on learnings: Assert both UI outcomes and backend/mock effects in E2E specs when relevant to ensure full-stack correctness.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 517 - 544, The
test currently allows logout to pass when either the UI shows the logged-out
marker (onWelcome) OR the auth token is cleared (hasToken), which lets the app
remain on a protected page while storage is cleared; change the assertion to
require both checks by asserting onWelcome && !hasToken instead of onWelcome ||
!hasToken, keeping the existing textExists calls, browser.pause, the
localStorage execute block, and the console.log that uses onWelcome and hasToken
so the test fails unless the visible logged-out UI (onWelcome) is true and the
token is cleared (!hasToken) at the same time.

---

239-246: ⚠️ Potential issue | 🟠 Major

performFullLogin() can now expose the recovery phrase on failure.

Because this flow always walks MnemonicStep now, the existing home-page dump below can log the seed phrase if onboarding stalls there. Gate the dump on textExists('Your Recovery Phrase') and redact when it is true. As per coding guidelines "Never log secrets, raw JWTs, API keys, or full PII—redact or omit sensitive fields".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 239 - 246, The
current post-login failure dump can expose the recovery phrase; before printing
or throwing, call textExists('Your Recovery Phrase') and if it returns true
redact sensitive content (or skip the dump) from the dumpAccessibilityTree
output; update the block that follows waitForHomePage (the failure branch that
calls dumpAccessibilityTree and logs it) to gate logging on textExists and
ensure any output is redacted when mnemonic text is present so secrets are never
written to logs.

app/test/e2e/specs/login-flow.spec.ts (3)

307-311: ⚠️ Potential issue | 🟠 Major

Redact these diagnostics once the walkthrough can reach the recovery-phrase screen.

After the new onboarding flow, a failed home check here can capture the mnemonic in the accessibility tree. Skip the dump or emit a redacted placeholder when Your Recovery Phrase is visible. As per coding guidelines "Never log secrets, raw JWTs, API keys, or full PII—redact or omit sensitive fields".

Also applies to: 371-378

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 307 - 311, The test is
potentially dumping the accessibility tree (and exposing the mnemonic) when the
recovery phrase screen is present; update the code around the browser.execute
calls in login-flow.spec.ts (the blocks that query the
checkbox/input[type="checkbox"] and the similar block at lines ~371-378) to
first check for the presence/visibility of the "Your Recovery Phrase" heading or
label and, if present, skip the dump or return a redacted placeholder (e.g.,
"REDACTED_RECOVERY_PHRASE_SCREEN") instead of returning DOM or accessibility
details; ensure any logging or test failure output uses that redacted value so
no secret mnemonic can be captured.

---

86-94: ⚠️ Potential issue | 🟠 Major

This local onboarding helper still short-circuits on slow renders.

clickFirstMatch() only makes one pass, and the walkthrough decides "overlay not visible" from a single probe. That can skip the onboarding path and the hadOnboardingWalkthrough bookkeeping even when the next screen appears a moment later.

Also applies to: 236-247

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 86 - 94, The helper
clickFirstMatch short-circuits because it only probes each candidate once;
change clickFirstMatch (and the duplicate at the 236-247 block) to poll for
matches until a provided overall timeout elapses: record a deadline (Date.now()
+ timeout) and in a loop iterate candidates repeatedly, calling textExists(text)
and then clickText(text) when true, breaking/returning the matched text; if none
become visible before the deadline, return null. Ensure you sleep/yield briefly
between iterations to avoid a tight spin.

---

123-129: ⚠️ Potential issue | 🟠 Major

These phases still share the same live session.

The onboarding, 401-path, and bypass cases all run against the window/storage created in before(). That makes failures order-dependent and weakens the negative-path assertions because they do not start from a guaranteed clean auth state. Based on learnings: Ensure each E2E spec is runnable in isolation without dependencies on other specs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 123 - 129, The tests
share a single live session created in the top-level before() so negative and
onboarding paths are order-dependent; change the setup to isolate each spec by
moving or duplicating the session/setup into a beforeEach (or adding teardown in
afterEach) so every test starts with a clean auth/storage/window state: call
startMockServer()/waitForApp() or create a fresh browser context per test, run
clearRequestLog(), reset hadOnboardingWalkthrough to false, and explicitly clear
localStorage/sessionStorage and cookies (or restart the app/browser context)
before each test so login, 401-path, and bypass cases run independently.

🧹 Nitpick comments (1)

app/test/wdio.conf.ts (1)

60-62: Type loosening trades type safety for cleaner code.

Switching from WebdriverIO.Capabilities[] to Record<string, unknown>[] and extending the config type removes @ts-expect-error suppressions but sacrifices compile-time validation of capability keys. This is a reasonable pragmatic choice given WebdriverIO's incomplete typing for platform-specific capabilities like tauri:options.

If stricter typing is desired later, consider defining a local interface that extends the base types:

interface TauriCapability {
  'tauri:options': { application: string };
}

interface Mac2Capability {
  platformName: string;
  'appium:automationName': string;
  'appium:app': string;
  'appium:showServerLogs': boolean;
}

type PlatformCapability = TauriCapability | Mac2Capability;

Also applies to: 82-82

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/wdio.conf.ts` around lines 60 - 62, The code loosened capability
typing by changing the return type of getPlatformCapabilities to Record<string,
unknown>[] which reduces compile-time safety; restore stricter typing by
defining local interfaces (e.g., TauriCapability and Mac2Capability) that
describe platform-specific keys like 'tauri:options', platformName, and
'appium:*' fields and use a union type (e.g., PlatformCapability) as the return
type of getPlatformCapabilities and where the config type was extended so you
keep type-safety for those capability shapes while avoiding `@ts-expect-error`
suppressions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/test/e2e/helpers/shared-flows.ts`:
- Around line 114-120: The billing-detection in navigateToBilling() is too loose
because textExists('Upgrade') matches Home's "Upgrade to Premium"; update the
checks that call textExists(...) (both inside the polling loop and the final
fallback) to look for billing-specific markers only — e.g., replace the generic
'Upgrade' check with a more specific string like 'Upgrade plan' or 'Manage
billing' or use an exact/whole-word match or a dedicated helper (e.g.,
textExistsExact or a regex with word boundaries) so navigateToBilling() only
returns success when billing UI elements (Current Plan, FREE as billing badge,
Manage billing/Upgrade plan) are actually present. Ensure you modify the checks
inside navigateToBilling() and the final fallback check to use the same
tightened matching logic.

In `@app/test/e2e/specs/conversations-web-channel-flow.spec.ts`:
- Around line 95-112: The test's first browser.execute returns only a boolean
(foundInput) while subsequent browser.execute blocks still query specifically
for textarea[placeholder*="Type a message"], so when the fallback element is
used the later typing/Enter dispatch targets nothing; change the first
browser.execute (the one that assigns foundInput) to return a unique locator
(e.g., a string selector or an attribute like data-test-id) or a reference
indicator for the actual element found (preferably the selector used:
'textarea[placeholder*="Type a message"]' or a generated selector for the
fallback like 'textarea' or '[contenteditable="true"]'), then pass that returned
selector/value into the following browser.execute calls so the same resolved
element is focused, receives the value injection and the Enter key dispatch;
update the analogous block at lines 121-145 too so both focus/typing/submit use
the same resolved selector returned by the initial browser.execute.

---

Duplicate comments:
In `@app/test/e2e/helpers/shared-flows.ts`:
- Around line 305-311: The failure dump printed after
walkOnboarding/waitForHomePage can leak the recovery phrase when the flow is
stuck on MnemonicStep; modify the error handling in the block that calls
waitForHomePage and dumpAccessibilityTree so that before logging or including
the accessibility tree you detect if the current screen/component is
MnemonicStep (or contains labels like "Your Recovery Phrase"/"recovery phrase")
and, if so, either replace sensitive fields in the dumped tree with a redacted
placeholder (e.g., "[REDACTED RECOVERY PHRASE]") or omit the tree entirely, then
log the safe/redacted message and throw the error — update the code around
walkOnboarding, waitForHomePage, and dumpAccessibilityTree accordingly.
- Around line 64-71: clickFirstMatch ignores its timeout and can return null too
quickly; change it so each candidate uses the provided timeout when waiting for
presence (call textExists(text, timeout) instead of textExists(text)) and pass
the same timeout into clickText; implement waiting/polling up to that timeout
per candidate rather than a single instantaneous probe. Also update
walkOnboarding to call clickFirstMatch with an appropriate per-step timeout (or
loop/retry until an overall timeout) so it doesn't give up after one probe;
reference the clickFirstMatch, textExists, clickText, and walkOnboarding symbols
when making these changes.

In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 517-544: The test currently allows logout to pass when either the
UI shows the logged-out marker (onWelcome) OR the auth token is cleared
(hasToken), which lets the app remain on a protected page while storage is
cleared; change the assertion to require both checks by asserting onWelcome &&
!hasToken instead of onWelcome || !hasToken, keeping the existing textExists
calls, browser.pause, the localStorage execute block, and the console.log that
uses onWelcome and hasToken so the test fails unless the visible logged-out UI
(onWelcome) is true and the token is cleared (!hasToken) at the same time.
- Around line 239-246: The current post-login failure dump can expose the
recovery phrase; before printing or throwing, call textExists('Your Recovery
Phrase') and if it returns true redact sensitive content (or skip the dump) from
the dumpAccessibilityTree output; update the block that follows waitForHomePage
(the failure branch that calls dumpAccessibilityTree and logs it) to gate
logging on textExists and ensure any output is redacted when mnemonic text is
present so secrets are never written to logs.

In `@app/test/e2e/specs/login-flow.spec.ts`:
- Around line 307-311: The test is potentially dumping the accessibility tree
(and exposing the mnemonic) when the recovery phrase screen is present; update
the code around the browser.execute calls in login-flow.spec.ts (the blocks that
query the checkbox/input[type="checkbox"] and the similar block at lines
~371-378) to first check for the presence/visibility of the "Your Recovery
Phrase" heading or label and, if present, skip the dump or return a redacted
placeholder (e.g., "REDACTED_RECOVERY_PHRASE_SCREEN") instead of returning DOM
or accessibility details; ensure any logging or test failure output uses that
redacted value so no secret mnemonic can be captured.
- Around line 86-94: The helper clickFirstMatch short-circuits because it only
probes each candidate once; change clickFirstMatch (and the duplicate at the
236-247 block) to poll for matches until a provided overall timeout elapses:
record a deadline (Date.now() + timeout) and in a loop iterate candidates
repeatedly, calling textExists(text) and then clickText(text) when true,
breaking/returning the matched text; if none become visible before the deadline,
return null. Ensure you sleep/yield briefly between iterations to avoid a tight
spin.
- Around line 123-129: The tests share a single live session created in the
top-level before() so negative and onboarding paths are order-dependent; change
the setup to isolate each spec by moving or duplicating the session/setup into a
beforeEach (or adding teardown in afterEach) so every test starts with a clean
auth/storage/window state: call startMockServer()/waitForApp() or create a fresh
browser context per test, run clearRequestLog(), reset hadOnboardingWalkthrough
to false, and explicitly clear localStorage/sessionStorage and cookies (or
restart the app/browser context) before each test so login, 401-path, and bypass
cases run independently.

---

Nitpick comments:
In `@app/test/wdio.conf.ts`:
- Around line 60-62: The code loosened capability typing by changing the return
type of getPlatformCapabilities to Record<string, unknown>[] which reduces
compile-time safety; restore stricter typing by defining local interfaces (e.g.,
TauriCapability and Mac2Capability) that describe platform-specific keys like
'tauri:options', platformName, and 'appium:*' fields and use a union type (e.g.,
PlatformCapability) as the return type of getPlatformCapabilities and where the
config type was extended so you keep type-safety for those capability shapes
while avoiding `@ts-expect-error` suppressions.

🪄 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

Run ID: 074d44ca-4ef7-47fc-8de6-3dc0f75e2ea6

📥 Commits

Reviewing files that changed from the base of the PR and between ea23d2b and 5c26bb6.

📒 Files selected for processing (11)

• app/test/e2e/helpers/shared-flows.ts
• app/test/e2e/specs/auth-access-control.spec.ts
• app/test/e2e/specs/card-payment-flow.spec.ts
• app/test/e2e/specs/conversations-web-channel-flow.spec.ts
• app/test/e2e/specs/crypto-payment-flow.spec.ts
• app/test/e2e/specs/gmail-flow.spec.ts
• app/test/e2e/specs/login-flow.spec.ts
• app/test/e2e/specs/notion-flow.spec.ts
• app/test/e2e/specs/tauri-commands.spec.ts
• app/test/e2e/specs/telegram-flow.spec.ts
• app/test/wdio.conf.ts

✅ Files skipped from review due to trivial changes (1)

• app/test/e2e/specs/tauri-commands.spec.ts

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

• app/test/e2e/specs/notion-flow.spec.ts
• app/test/e2e/specs/crypto-payment-flow.spec.ts
• app/test/e2e/specs/gmail-flow.spec.ts
• app/test/e2e/specs/card-payment-flow.spec.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

navigateToBilling() can still report success while the app is on Home.

textExists() is a contains match, so bare Upgrade also matches Home’s Upgrade to Premium. Both the initial loop and the final fallback check can therefore succeed without any billing-only marker on screen, and downstream specs can end up clicking the wrong CTA.

🧭 Tighten the billing check

-    hasBilling =
-      (await textExists('Current Plan')) ||
-      (await textExists('FREE')) ||
-      (await textExists('Upgrade'));
+    const currentHash = await browser.execute(() => window.location.hash);
+    hasBilling =
+      currentHash.startsWith('#/settings/billing') &&
+      ((await textExists('Current Plan')) ||
+        (await textExists('FREE')) ||
+        (await textExists('Manage')) ||
+        (await textExists('Billing & Usage')));
@@
-  const finalCheck =
-    (await textExists('Current Plan')) ||
-    (await textExists('FREE')) ||
-    (await textExists('Upgrade'));
+  const finalHash = await browser.execute(() => window.location.hash);
+  const finalCheck =
+    finalHash.startsWith('#/settings/billing') &&
+    ((await textExists('Current Plan')) ||
+      (await textExists('FREE')) ||
+      (await textExists('Manage')) ||
+      (await textExists('Billing & Usage')));
   if (!finalCheck) {
-    const finalHash = await browser.execute(() => window.location.hash);
     const tree = await dumpAccessibilityTree();

Also applies to: 155-160

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/helpers/shared-flows.ts` around lines 114 - 120, The
billing-detection in navigateToBilling() is too loose because
textExists('Upgrade') matches Home's "Upgrade to Premium"; update the checks
that call textExists(...) (both inside the polling loop and the final fallback)
to look for billing-specific markers only — e.g., replace the generic 'Upgrade'
check with a more specific string like 'Upgrade plan' or 'Manage billing' or use
an exact/whole-word match or a dedicated helper (e.g., textExistsExact or a
regex with word boundaries) so navigateToBilling() only returns success when
billing UI elements (Current Plan, FREE as billing badge, Manage billing/Upgrade
plan) are actually present. Ensure you modify the checks inside
navigateToBilling() and the final fallback check to use the same tightened
matching logic.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

The fallback input path never reaches the typing or submission code.

foundInput returns true for any textarea or [contenteditable="true"], but the next two browser.execute() blocks only query textarea[placeholder*="Type a message"]. If the placeholder differs, the test focuses one element and types into nothing. Reuse the same resolved selector/element for focus, value injection, and Enter dispatch.

Also applies to: 121-145

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/conversations-web-channel-flow.spec.ts` around lines 95 -
112, The test's first browser.execute returns only a boolean (foundInput) while
subsequent browser.execute blocks still query specifically for
textarea[placeholder*="Type a message"], so when the fallback element is used
the later typing/Enter dispatch targets nothing; change the first
browser.execute (the one that assigns foundInput) to return a unique locator
(e.g., a string selector or an attribute like data-test-id) or a reference
indicator for the actual element found (preferably the selector used:
'textarea[placeholder*="Type a message"]' or a generated selector for the
fallback like 'textarea' or '[contenteditable="true"]'), then pass that returned
selector/value into the following browser.execute calls so the same resolved
element is focused, receives the value injection and the Enter key dispatch;
update the analogous block at lines 121-145 too so both focus/typing/submit use
the same resolved selector returned by the initial browser.execute.

[coderabbitai]

🧹 Nitpick comments (3)

app/test/e2e/specs/login-flow.spec.ts (3)

391-425: Phase 4 tests verify backend rejection but not UI outcome.

These tests confirm the mock returns 401 for expired/invalid tokens but don't assert the app's UI state (e.g., showing login screen or clearing auth). The commit message explains this is intentional due to Tauri's single-instance architecture where Redux state persists.

Consider adding a brief comment documenting this limitation so future maintainers understand why UI assertions are absent:

 it('expired token triggers consume call that returns 401', async () => {
   // Note: The app is already authenticated from Phase 1-3. In a single-instance
   // Tauri desktop app, we cannot fully reset the in-memory Redux state between
   // tests. This test verifies that the expired token deep link triggers the
-  // consume call and the mock rejects it with 401.
+  // consume call and the mock rejects it with 401. UI assertions are omitted
+  // because the prior session's Redux state remains in memory.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 391 - 425, The tests
'expired token triggers consume call that returns 401' and 'invalid token
triggers consume call that returns 401' only assert the backend consume call and
mock 401 responses but do not verify UI/auth state due to Tauri single-instance
Redux persistence; add a short inline comment above each it(...) (or a shared
comment above both tests) referencing this limitation and explaining why UI
assertions (e.g., checking for login screen or cleared auth) are intentionally
omitted and that the test only ensures the deep link handler attempted the
consume call and the mock rejected it.

---

119-129: Acknowledged: tests share session state by design.

The suite relies on sequential execution where Phase 4/5 reuse the authenticated session from Phase 1-3. While this violates the "runnable in isolation" guideline, the commit message explains this is intentional for single-instance Tauri apps where in-memory Redux state cannot be fully reset between tests.

Consider adding a comment in the test file documenting this constraint so future maintainers understand the design:

+// NOTE: This suite runs sequentially and tests share session state.
+// Full isolation is not possible in a single-instance Tauri desktop app
+// where in-memory Redux state persists across deep link invocations.
 let hadOnboardingWalkthrough = false;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 119 - 129, Add an
explicit comment near the hadOnboardingWalkthrough variable (or immediately
before the describe('Login flow — complete with mock data (Linux)') block)
stating that tests intentionally share session state and must run sequentially
because this is a single-instance Tauri app with in-memory Redux state that
cannot be fully reset between test phases ( Phases 1–5 reuse the authenticated
session ), so the suite is not runnable in isolation by design; reference
hadOnboardingWalkthrough and the Phase 4/5 dependency to make the constraint
clear for future maintainers.

---

259-266: Consider using distinct variable names instead of bare blocks.

The bare block { } around each step prevents variable shadowing for clicked, but it's an uncommon pattern that may confuse readers. Using distinct names (clickedLocalAI, clickedPermissions, etc.) would be more explicit.

-    // Step 1: LocalAIStep — only has "Use Local Models" button now
-    {
-      const clicked = await clickFirstMatch(['Use Local Models', 'Continue'], 10_000);
-      if (clicked) {
-        console.log(`[LoginFlow] LocalAIStep: clicked "${clicked}"`);
-        await browser.pause(2_000);
-      }
+    // Step 1: LocalAIStep — only has "Use Local Models" button now
+    const clickedLocalAI = await clickFirstMatch(['Use Local Models', 'Continue'], 10_000);
+    if (clickedLocalAI) {
+      console.log(`[LoginFlow] LocalAIStep: clicked "${clickedLocalAI}"`);
+      await browser.pause(2_000);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/test/e2e/specs/login-flow.spec.ts` around lines 259 - 266, The bare
blocks around each step are used to avoid shadowing the variable clicked;
replace them with distinct, descriptive variable names (e.g., clickedLocalAI for
the LocalAIStep) instead of using `{ }` and the generic clicked identifier.
Update the LocalAIStep code that calls clickFirstMatch (and similar steps in the
LoginFlow spec) to assign results to unique variables like clickedLocalAI,
clickedPermissions, etc., log those names (e.g., `[LoginFlow] LocalAIStep:
clicked "${clickedLocalAI}"`), and remove the unnecessary bare blocks to improve
clarity.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/test/e2e/specs/login-flow.spec.ts`:
- Around line 391-425: The tests 'expired token triggers consume call that
returns 401' and 'invalid token triggers consume call that returns 401' only
assert the backend consume call and mock 401 responses but do not verify UI/auth
state due to Tauri single-instance Redux persistence; add a short inline comment
above each it(...) (or a shared comment above both tests) referencing this
limitation and explaining why UI assertions (e.g., checking for login screen or
cleared auth) are intentionally omitted and that the test only ensures the deep
link handler attempted the consume call and the mock rejected it.
- Around line 119-129: Add an explicit comment near the hadOnboardingWalkthrough
variable (or immediately before the describe('Login flow — complete with mock
data (Linux)') block) stating that tests intentionally share session state and
must run sequentially because this is a single-instance Tauri app with in-memory
Redux state that cannot be fully reset between test phases ( Phases 1–5 reuse
the authenticated session ), so the suite is not runnable in isolation by
design; reference hadOnboardingWalkthrough and the Phase 4/5 dependency to make
the constraint clear for future maintainers.
- Around line 259-266: The bare blocks around each step are used to avoid
shadowing the variable clicked; replace them with distinct, descriptive variable
names (e.g., clickedLocalAI for the LocalAIStep) instead of using `{ }` and the
generic clicked identifier. Update the LocalAIStep code that calls
clickFirstMatch (and similar steps in the LoginFlow spec) to assign results to
unique variables like clickedLocalAI, clickedPermissions, etc., log those names
(e.g., `[LoginFlow] LocalAIStep: clicked "${clickedLocalAI}"`), and remove the
unnecessary bare blocks to improve clarity.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f769e478-6f21-43bd-8dd5-51a3d4372366

📥 Commits

Reviewing files that changed from the base of the PR and between 5c26bb6 and 4ffe7fe.

📒 Files selected for processing (1)

• app/test/e2e/specs/login-flow.spec.ts