test(e2e): unify driver onto Appium Chromium attached to CEF CDP#1696
Merged
senamakel merged 13 commits intoMay 14, 2026
Conversation
Collapses the per-platform driver split (Appium Mac2 on macOS,
tauri-driver on Linux/Windows) onto a single backend: Appium Chromium
driver attached to CEF's already-enabled remote-debugging port (:19222,
configured in app/src-tauri/src/lib.rs:1556). One session per run, all
specs sequential — eliminates the per-spec Appium + app restart cost.
- wdio.conf.ts: single Chromium capability with debuggerAddress.
- scripts/e2e-run-session.sh: new unified runner (launch CEF, wait for
CDP, start Appium, run wdio).
- scripts/e2e-run-spec.sh: thin shim delegating to the session runner.
- helpers/platform.ts: shim — isTauriDriver/supportsExecuteScript now
always true, isMac2 always false, so existing branched specs uniformly
take the DOM path Chromium driver supports on every platform.
- helpers/app-helpers.ts: waitForApp does a real readiness check.
- package.json: add test:e2e:session{,:full} scripts.
…uri-cli Three follow-ups on top of the unified-driver foundation, validated by a green smoke run on macOS: - e2e-build.sh: call `pnpm tauri:ensure` and use `cargo tauri build` (not `pnpm exec tauri build`) so the .app bundle ships the CEF framework. Without this, the built binary panics at startup inside `cef::library_loader::LibraryLoader::new`. - e2e-run-session.sh: parse Chromium version from /json/version, download the matching chromedriver from Chrome for Testing into a workspace cache, and export E2E_CHROMEDRIVER_PATH. Without this, Appium's bundled chromedriver (currently v148) refuses to attach to CEF Chromium 146. - wdio.conf.ts: use the real host platformName (mac/linux/windows — the appium-chromium-driver supports those, not "chromium"), and pass the chromedriver path via `appium:executable` (the driver's actual cap name; `chromedriverExecutable` is silently dropped).
Three follow-on fixes after the unified driver landed. With these, the
smoke spec passes in ~1s on macOS via the new pipeline (was ~21s before
because chromedriver was attaching to the wrong CDP target).
- e2e-run-session.sh: wait for the `tauri.localhost` page target to
appear in CEF's /json/list, then close any `about:` page targets
(the CEF prewarm child-webview slot) via /json/close/<id> so
chromedriver's debuggerAddress attach binds to the real app. Without
this, chromedriver attached to the prewarm `about:blank` and the
session died as soon as CEF GC'd that target.
- wdio.conf.ts: defensive `before:` hook switches to the main webview
in case more than one page target survives the runner-side prune.
- core-rpc.ts: drop the WebView path that did
`await import('@tauri-apps/api/core')` inside `browser.execute`. The
dynamic specifier only resolved under tauri-driver's renderer import
map; under direct CDP it 404s with "Failed to resolve module
specifier". The Node-side HTTP path (bearer token from tmpdir +
port probe) is driver-agnostic and is now the only path.
- .gitignore: ignore the per-version chromedriver cache the runner
populates from Chrome for Testing.
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR unifies E2E testing across Linux, macOS, and Windows using a single Appium Chromium driver attached to CEF's remote-debugging protocol (CDP). It adds a session orchestration script, updates build and WDIO config to attach to CDP, simplifies test helpers, and updates CI workflows and scripts to use the unified flow. ChangesAppium Chromium E2E Unification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…unner Every E2E job now runs the same recipe: install Appium 3 + the chromium driver, build the CEF app, and invoke `app/scripts/e2e-run-session.sh`. The script handles CDP attach, prewarm-target pruning, chromedriver version pinning to CEF's Chromium, Appium startup, and WDIO invocation. - e2e.yml: Linux/macOS/Windows now share an identical recipe. Linux runs inside Xvfb. Linux + Windows are continue-on-error until we have a green CI run; macOS is the gating signal (smoke spec validated locally in ~1s on the unified driver). The old "build-only" workarounds for Linux and Windows are gone — they couldn't actually run specs because tauri-driver doesn't speak to CEF. Now they can. - e2e-agent-review.yml: same flip — drop tauri-driver, install Appium chromium driver, call e2e-run-session.sh. mega-flow runs but is non-blocking everywhere: phase 2 work (handling CEF window recreation that kills the chromedriver session) is needed before it gates merges.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Member
Author
|
@coderabbitai reviwe |
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/scripts/e2e-run-session.sh (2)
176-201: ⚡ Quick winPipeline failures indicate CEF app crashes before CDP is available.
The CI logs show "CEF app exited before CDP came up" on both Windows and Linux. The script's error handling is correct (lines 192-195 detect early exit), but the root cause appears to be an app/build issue rather than a script defect.
Consider adding more diagnostic output when the app exits early to help debug:
🔍 Add diagnostic output for early app exit
if ! kill -0 "$APP_PID" 2>/dev/null; then echo "ERROR: CEF app exited before CDP came up. See $APP_LOG" >&2 + echo "--- Last 50 lines of app log ---" >&2 + tail -50 "$APP_LOG" 2>/dev/null >&2 || true + echo "--- End of app log ---" >&2 exit 1 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/scripts/e2e-run-session.sh` around lines 176 - 201, When the script detects the app exited early (the kill -0 "$APP_PID" check in the CDP wait loop) add diagnostic steps to capture why: immediately wait for the process to reap and capture its exit code (use wait on APP_PID), print a brief tail of APP_LOG (e.g., last ~200 lines), and print lightweight system state (e.g., ps/pgrep for APP_BIN and free disk space) so CI logs include the app exit code and recent app logs; reference APP_PID, APP_LOG, CEF_CDP_PORT and CDP_VERSION_JSON to locate the changes in the CDP-wait loop.
128-138: 💤 Low valueConsider using
findinstead oflsfor glob expansion.Static analysis (SC2012) flags this:
lscan misbehave with non-alphanumeric filenames. While unlikely in this controlled context,findis more robust.♻️ Suggested refactor
-DIST_JS="$(ls dist/assets/index-*.js 2>/dev/null | head -1)" +DIST_JS="$(find dist/assets -maxdepth 1 -name 'index-*.js' -print -quit 2>/dev/null)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/scripts/e2e-run-session.sh` around lines 128 - 138, The use of ls to populate DIST_JS (ls dist/assets/index-*.js | head -1) is flagged by SC2012; replace that pipeline with a robust find-based assignment that safely locates the first matching file (pattern dist/assets/index-*.js) and assigns it to DIST_JS, preserving the subsequent existence check and the grep check for "127.0.0.1:${E2E_MOCK_PORT}" against DIST_JS; ensure the find invocation returns a single path (or empty) so the rest of the script (the if [ -z "$DIST_JS" ] and the grep check) continues to work as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/scripts/e2e-run-session.sh`:
- Around line 85-90: The script's OS case handling is missing Windows
(MINGW/MSYS/CYGWIN/Windows_NT) cleanup so OpenHuman processes aren't killed on
Windows; update the case "$OS" in the e2e-run-session.sh to add a Windows branch
(match MINGW*|MSYS*|CYGWIN*|Windows_NT) that invokes the Windows process-kill
command (e.g. taskkill /F /IM OpenHuman.exe 2>/dev/null || true) or a
tasklist/taskkill combo that targets processes whose image or command line
contains "OpenHuman", keeping the existing pkill usage for Darwin and Linux and
preserving the 2>/dev/null || true behavior.
In `@app/test/e2e/helpers/app-helpers.ts`:
- Around line 23-29: The current blanket catch around waitForAppReady(15_000)
masks all errors; change it to catch the error into a variable, inspect it, and
only swallow/fallback to await browser.pause(5_000) for genuine
readiness/timeouts (e.g., check for a specific TimeoutError type or
error.message that indicates not-ready), otherwise rethrow the error so
execution/session/DOM failures surface; update the catch block in the helper
where waitForAppReady is called to use catch (err) { if (isReadinessError(err))
await browser.pause(5_000); else throw err; } (or equivalent check) referencing
waitForAppReady and the catch block in app-helpers.ts.
---
Nitpick comments:
In `@app/scripts/e2e-run-session.sh`:
- Around line 176-201: When the script detects the app exited early (the kill -0
"$APP_PID" check in the CDP wait loop) add diagnostic steps to capture why:
immediately wait for the process to reap and capture its exit code (use wait on
APP_PID), print a brief tail of APP_LOG (e.g., last ~200 lines), and print
lightweight system state (e.g., ps/pgrep for APP_BIN and free disk space) so CI
logs include the app exit code and recent app logs; reference APP_PID, APP_LOG,
CEF_CDP_PORT and CDP_VERSION_JSON to locate the changes in the CDP-wait loop.
- Around line 128-138: The use of ls to populate DIST_JS (ls
dist/assets/index-*.js | head -1) is flagged by SC2012; replace that pipeline
with a robust find-based assignment that safely locates the first matching file
(pattern dist/assets/index-*.js) and assigns it to DIST_JS, preserving the
subsequent existence check and the grep check for "127.0.0.1:${E2E_MOCK_PORT}"
against DIST_JS; ensure the find invocation returns a single path (or empty) so
the rest of the script (the if [ -z "$DIST_JS" ] and the grep check) continues
to work as before.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db08a16c-fbea-4249-b8f9-76a080f34d5d
📒 Files selected for processing (11)
.github/workflows/e2e-agent-review.yml.github/workflows/e2e.ymlapp/.gitignoreapp/package.jsonapp/scripts/e2e-build.shapp/scripts/e2e-run-session.shapp/scripts/e2e-run-spec.shapp/test/e2e/helpers/app-helpers.tsapp/test/e2e/helpers/core-rpc.tsapp/test/e2e/helpers/platform.tsapp/test/wdio.conf.ts
…t at runtime
- macOS workflow: install x86_64-apple-darwin target. The vendored tauri-cli's
build.rs (tauri-bundler/build.rs:134) compiles a CEF helper for that target
to support universal binaries, even on arm64 hosts. macos-latest is arm64
and the target wasn't installed → "can't find crate for `core`".
- e2e-run-session.sh: on Linux + Windows, find the CEF distribution that the
vendored tauri-cli cached under $CEF_PATH/<version>/cef_<os>_<arch>/ and
prepend it to LD_LIBRARY_PATH / PATH before launching the binary. The bare
--no-bundle build has no co-located libcef.so/libcef.dll, so the OS loader
couldn't find them and the app exited before CDP came up.
- e2e-run-session.sh: log paths now use $RUNNER_TEMP / $TMPDIR rather than
hard-coded /tmp. Windows runners don't have /tmp so the artifact-upload
glob never matched anything.
- e2e.yml: artifact globs use ${{ runner.temp }} to match the new log paths.
The upload-artifact path mismatch on docker container runners means the $RUNNER_TEMP-based artifact glob doesn't pick up logs created inside the container view (/__w/_temp). Print the log to stderr so the CI run page has the diagnosis without needing artifact archaeology.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/scripts/e2e-run-session.sh (1)
85-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the missing Windows process cleanup before probing CDP.
This
case "$OS"still leavesOpenHuman.exerunning on Windows, so the laterhttp://127.0.0.1:${CEF_CDP_PORT}/json/versioncheck can attach to a stale CEF instance instead of the one this script just launched.🔧 Minimal fix
echo "[runner] Killing any running OpenHuman instances..." case "$OS" in Darwin) pkill -f "OpenHuman" 2>/dev/null || true ;; Linux) pkill -f "OpenHuman" 2>/dev/null || true ;; + MINGW*|MSYS*|CYGWIN*|Windows_NT) + taskkill.exe //F //IM "OpenHuman.exe" >/dev/null 2>&1 || true + ;; esac sleep 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/scripts/e2e-run-session.sh` around lines 85 - 90, The script's OS cleanup misses Windows so lingering OpenHuman.exe can block the CDP probe; update the case handling for the variable OS in e2e-run-session.sh (the block that matches Darwin/Linux) to also handle Windows (e.g., "Windows" or "MINGW"* as used by your OS detection) and run a Windows-safe termination command such as taskkill /IM "OpenHuman.exe" /F 2>/dev/null || true (or an equivalent Stop-Process invocation for PowerShell environments), ensuring you still silence errors and fall back to || true so the script continues.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/scripts/e2e-run-session.sh`:
- Around line 373-389: The readiness probe can succeed against a stale Appium if
the newly started process failed with EADDRINUSE; after starting Appium (the
block using APPIUM_BIN, APPIUM_PORT, APPIUM_LOG and APPIUM_PID) add a quick
verification step: wait a second, assert the background PID ($APPIUM_PID) is
alive (kill -0) and then verify that the listener on APPIUM_PORT is owned by
that PID (use lsof -i :$APPIUM_PORT -sTCP:LISTEN -n -P or ss/netstat fallback)
before accepting the readiness curl; if the PID is dead or the port is owned by
a different PID, print an error and exit non‑zero so we fail fast instead of
driving the wrong Appium instance.
- Around line 190-217: The script currently picks a CEF_DIST_DIR with head -1
which silently chooses an arbitrary cached distribution when multiple exist;
update the Linux and Windows branches (where CEF_DIST_DIR is computed and
exported to LD_LIBRARY_PATH/PATH, and where cygpath/WIN_CEF_DIST is used) to
detect multiple matching directories and either (a) select deterministically the
newest distribution (e.g., sort by modification time) or (b) fail fast with a
clear error/warning asking the runner to clear the cache; implement this by
replacing the plain head -1 selection logic with a deterministic ls -dt ... |
head -1 (or by checking if more than one match exists and exiting non-zero with
an explicit message) so the script no longer silently picks the wrong libcef.
---
Duplicate comments:
In `@app/scripts/e2e-run-session.sh`:
- Around line 85-90: The script's OS cleanup misses Windows so lingering
OpenHuman.exe can block the CDP probe; update the case handling for the variable
OS in e2e-run-session.sh (the block that matches Darwin/Linux) to also handle
Windows (e.g., "Windows" or "MINGW"* as used by your OS detection) and run a
Windows-safe termination command such as taskkill /IM "OpenHuman.exe" /F
2>/dev/null || true (or an equivalent Stop-Process invocation for PowerShell
environments), ensuring you still silence errors and fall back to || true so the
script continues.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 87764d31-200e-4e70-81db-e0fa962b6a41
📒 Files selected for processing (1)
app/scripts/e2e-run-session.sh
The openhuman_ci docker image runs as root, and Chromium refuses to start under uid 0 without --no-sandbox (crbug.com/638180). With the inline app log now in the CI run output, the failure surface was immediately visible: ERROR:zygote_host_impl_linux.cc:101] Running as root without --no-sandbox is not supported. Detect uid 0 on Linux and append --no-sandbox to the binary's argv. The flag is intentionally NOT applied on macOS/Windows where the runner is a regular user and the sandbox should stay enabled.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
app/scripts/e2e-run-session.sh (3)
85-89:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the Windows cleanup branch here too.
This block only kills stale processes on Darwin/Linux, so reruns on Git Bash/MSYS can leave
OpenHuman.exealive and keep the CDP/app ports busy.🔧 Minimal fix
echo "[runner] Killing any running OpenHuman instances..." case "$OS" in Darwin) pkill -f "OpenHuman" 2>/dev/null || true ;; Linux) pkill -f "OpenHuman" 2>/dev/null || true ;; + MINGW*|MSYS*|CYGWIN*|Windows_NT) + taskkill //F //IM "OpenHuman.exe" 2>/dev/null || true + ;; esac sleep 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/scripts/e2e-run-session.sh` around lines 85 - 89, The cleanup only handles Darwin/Linux so stale Windows Git Bash/MSYS processes like OpenHuman.exe aren't killed; update the case on $OS to add a Windows branch (match MINGW*|MSYS*|CYGWIN*|Windows) and invoke the Windows process killer (taskkill /F /IM OpenHuman.exe or equivalent via cmd) redirecting errors to null and ensuring the command returns success (|| true) just like the existing Darwin/Linux branches; look for the pkill -f "OpenHuman" lines and the case "$OS" block to insert the Windows branch.
190-217:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't silently pick the first cached CEF distribution.
head -1makes warmed runners nondeterministic once multiple CEF versions exist underCEF_PATH, so Linux/Windows can load the wronglibcefand fail only intermittently.🔧 Minimal fix
case "$OS" in Linux) - CEF_DIST_DIR="$(ls -d "$CEF_PATH"/*/cef_linux_* 2>/dev/null | head -1)" + mapfile -t CEF_CANDIDATES < <(find "$CEF_PATH" -mindepth 2 -maxdepth 2 -type d -name 'cef_linux_*' | sort) + if [ "${`#CEF_CANDIDATES`[@]}" -gt 1 ]; then + echo "ERROR: multiple CEF Linux distributions found under $CEF_PATH; clear the cache or pin CEF_PATH." >&2 + exit 1 + fi + CEF_DIST_DIR="${CEF_CANDIDATES[0]:-}" if [ -n "$CEF_DIST_DIR" ] && [ -d "$CEF_DIST_DIR" ]; then export LD_LIBRARY_PATH="$CEF_DIST_DIR:${LD_LIBRARY_PATH:-}" echo "[runner] LD_LIBRARY_PATH includes: $CEF_DIST_DIR" else echo "[runner] Warning: no CEF Linux distribution found under $CEF_PATH — libcef.so may fail to load." fi ;; MINGW*|MSYS*|CYGWIN*|Windows_NT) - CEF_DIST_DIR="$(ls -d "$CEF_PATH"/*/cef_windows_* 2>/dev/null | head -1)" + mapfile -t CEF_CANDIDATES < <(find "$CEF_PATH" -mindepth 2 -maxdepth 2 -type d -name 'cef_windows_*' | sort) + if [ "${`#CEF_CANDIDATES`[@]}" -gt 1 ]; then + echo "ERROR: multiple CEF Windows distributions found under $CEF_PATH; clear the cache or pin CEF_PATH." >&2 + exit 1 + fi + CEF_DIST_DIR="${CEF_CANDIDATES[0]:-}" if [ -n "$CEF_DIST_DIR" ] && [ -d "$CEF_DIST_DIR" ]; then if command -v cygpath >/dev/null 2>&1; then WIN_CEF_DIST="$(cygpath -w "$CEF_DIST_DIR")"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/scripts/e2e-run-session.sh` around lines 190 - 217, The script currently uses head -1 to pick the first CEF distribution which is nondeterministic when multiple versions exist; change the CEF_DIST_DIR resolution for both Linux and Windows branches (the lines setting CEF_DIST_DIR via ls -d "$CEF_PATH"/*/cef_linux_* and ls -d "$CEF_PATH"/*/cef_windows_*) to deterministically pick the intended distribution (for example, pipe the ls output to sort -V and use tail -n1 to select the highest/version-sorted entry) so LD_LIBRARY_PATH (CEF_DIST_DIR) on Linux and PATH (CEF_DIST_DIR / WIN_CEF_DIST) on Windows always point to a consistent, latest CEF directory before the existing existence checks and exports.
386-402:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast if
APPIUM_PORTalready belongs to another server.The readiness loop accepts any
/statusresponder on that port. If the new Appium dies withEADDRINUSE, this can still pass against a stale instance and run the suite against the wrong server.🔧 Minimal fix
APPIUM_LOG="$LOG_DIR/appium-e2e-${LOG_SUFFIX}.log" +if curl -sf "http://127.0.0.1:$APPIUM_PORT/status" >/dev/null 2>&1; then + echo "ERROR: Appium is already listening on port $APPIUM_PORT. Stop the stale server or set APPIUM_PORT." >&2 + exit 1 +fi + echo "[runner] Starting Appium on port $APPIUM_PORT" echo "[runner] Appium logs: $APPIUM_LOG" "$APPIUM_BIN" --port "$APPIUM_PORT" --relaxed-security > "$APPIUM_LOG" 2>&1 & APPIUM_PID=$! for i in $(seq 1 30); do + if ! kill -0 "$APPIUM_PID" 2>/dev/null; then + echo "ERROR: Appium exited before becoming ready. Appium log follows:" >&2 + cat "$APPIUM_LOG" >&2 || true + exit 1 + fi if curl -sf "http://127.0.0.1:$APPIUM_PORT/status" >/dev/null 2>&1; then echo "[runner] Appium is ready." break fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/scripts/e2e-run-session.sh` around lines 386 - 402, The readiness loop can be fooled by a prior server on APPIUM_PORT; after launching Appium (using APPIUM_BIN to start and setting APPIUM_PID and APPIUM_LOG), immediately check whether the background process actually bound the port and/or died with EADDRINUSE before accepting the /status response: capture the start exit state by waiting a short moment (e.g. sleep 0.1) and if the process is no longer running (check APPIUM_PID), or if APPIUM_LOG contains EADDRINUSE, fail fast; alternatively verify the socket owner matches APPIUM_PID (use lsof/ss to confirm the listening PID is APPIUM_PID for APPIUM_PORT) and if not, print an error and exit 1 rather than proceeding to the /status loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@app/scripts/e2e-run-session.sh`:
- Around line 85-89: The cleanup only handles Darwin/Linux so stale Windows Git
Bash/MSYS processes like OpenHuman.exe aren't killed; update the case on $OS to
add a Windows branch (match MINGW*|MSYS*|CYGWIN*|Windows) and invoke the Windows
process killer (taskkill /F /IM OpenHuman.exe or equivalent via cmd) redirecting
errors to null and ensuring the command returns success (|| true) just like the
existing Darwin/Linux branches; look for the pkill -f "OpenHuman" lines and the
case "$OS" block to insert the Windows branch.
- Around line 190-217: The script currently uses head -1 to pick the first CEF
distribution which is nondeterministic when multiple versions exist; change the
CEF_DIST_DIR resolution for both Linux and Windows branches (the lines setting
CEF_DIST_DIR via ls -d "$CEF_PATH"/*/cef_linux_* and ls -d
"$CEF_PATH"/*/cef_windows_*) to deterministically pick the intended distribution
(for example, pipe the ls output to sort -V and use tail -n1 to select the
highest/version-sorted entry) so LD_LIBRARY_PATH (CEF_DIST_DIR) on Linux and
PATH (CEF_DIST_DIR / WIN_CEF_DIST) on Windows always point to a consistent,
latest CEF directory before the existing existence checks and exports.
- Around line 386-402: The readiness loop can be fooled by a prior server on
APPIUM_PORT; after launching Appium (using APPIUM_BIN to start and setting
APPIUM_PID and APPIUM_LOG), immediately check whether the background process
actually bound the port and/or died with EADDRINUSE before accepting the /status
response: capture the start exit state by waiting a short moment (e.g. sleep
0.1) and if the process is no longer running (check APPIUM_PID), or if
APPIUM_LOG contains EADDRINUSE, fail fast; alternatively verify the socket owner
matches APPIUM_PID (use lsof/ss to confirm the listening PID is APPIUM_PID for
APPIUM_PORT) and if not, print an error and exit 1 rather than proceeding to the
/status loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2effb35f-8c36-4118-a8ac-60d8a72c8df4
📒 Files selected for processing (1)
app/scripts/e2e-run-session.sh
…ndle Two new failures surfaced now that the previous blockers cleared. Inline log dump made both diagnoses cheap: - Linux: CEF zygote was crashing with "Failed global descriptor lookup: 7" and a flurry of dbus errors after --no-sandbox let it past the root check. Add --disable-dev-shm-usage (docker /dev/shm is too small), --disable-gpu, and --no-zygote so Chromium stops asking for a dbus session bus that isn't there. - macOS: the .app bundle launched and exited within ~2 seconds writing zero bytes to stdout/stderr. Classic "the OS killed the helper subprocess before it could log" symptom on unsigned bundles. Adhoc-sign with `codesign --force --deep --sign -` after build so the loader stops rejecting the CEF helpers.
With the previous fixes in place CDP actually came up on Linux —
"DevTools listening on ws://127.0.0.1:19222/devtools/browser/..." —
but the host process then panicked inside Tauri's single-instance plugin:
thread 'main' panicked at plugins/single-instance/.../linux.rs:57:
Result::unwrap() on Err(Address("unsupported transport 'disabled'"))
The openhuman_ci container ships with DBUS_SESSION_BUS_ADDRESS=disabled:.
zbus refuses that transport, so the plugin's setup crashes the host.
Start a real session bus with `dbus-launch` and inherit its address for
the rest of the runner. Skip on macOS/Windows where a session bus
already exists.
macOS run on d0102d0 failed with "rustup-init[EXE]: unexpected argument 'install' found" from `cargo install --locked --path .../tauri-cli`. The Swatinem cache from a prior failed run had restored a poisoned ~/.cargo state where `cargo` resolved to rustup-init. Two fixes layered for resilience: - Bump the rust-cache key to e2e-macos-unified-v2 so the bad state isn't restored on this run. - Add `rustup default 1.93.0 && which cargo && cargo --version` step after toolchain install so a future cargo/rustup-init shadow is caught in CI with a clear error instead of silently breaking ensure-tauri-cli.
Massive progress on Linux with the dbus + CEF-args + lib-path fixes — the app launched, CDP came up, prewarm pruned, chromedriver downloaded. Only `unzip` was missing in the openhuman_ci docker image: app/scripts/e2e-run-session.sh: line 402: unzip: command not found Python 3 is already required by the runner (used to parse /json/list), so use stdlib zipfile when `unzip` isn't on PATH. Avoids an apt-get install for one binary.
Runner uses `set -euo pipefail`, so expanding "${APP_ARGS[@]}" when the
array is empty (macOS/Windows path — no extra CEF flags needed) errors
with "APP_ARGS[@]: unbound variable" and the app never launches.
Use `${APP_ARGS[@]+"${APP_ARGS[@]}"}` to expand only when set, so macOS
gets the bare binary launch it wants and Linux still gets --no-sandbox
et al.
Four fixes flagged on PR tinyhumansai#1696's first CodeRabbit pass: - e2e-run-session.sh: add Windows process cleanup (`taskkill /F /IM OpenHuman.exe`). The Darwin/Linux pkill branches didn't have a Windows equivalent, so stale instances would survive across runs on Windows. - helpers/app-helpers.ts: stop swallowing non-timeout errors in `waitForApp()`. The previous blanket `catch` masked session/DOM failures behind a blind 5s pause. Re-throw anything that isn't the specific "waitForAppReady timed out" message so real failures surface. - e2e-run-session.sh: error out when CEF_PATH contains more than one cef_<os>_<arch> directory instead of silently picking the first via `ls | head -1`. After a CEF upgrade the wrong libcef would load and fail only on warmed runners. - e2e-run-session.sh: fail fast when APPIUM_PORT is already in use, and check that the Appium PID is still alive on each poll. Without this, a stale Appium passes the /status probe while our just-launched process dies with EADDRINUSE — and the tests drive the wrong instance.
12 tasks
This was referenced May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
19222, configured inapp/src-tauri/src/lib.rs:1556).app/scripts/e2e-run-session.sh; legacye2e-run-spec.shnow a thin shim so existingpnpm test:e2e:cron-jobsetc. keep working.app/test/e2e/.cache/chromedriver/<version>-<platform>/.Problem
The harness was split into two driver backends with two separate code paths:
tauri-driver(W3C WebDriver, full DOM access)..appbundle through Appium Mac2 (XCUITest accessibility tree, no DOM, noexecuteScript).Every spec branched on
isTauriDriver()andisMac2(), and helpers likeelement-helpers.tscarried duplicate XPath-over-accessibility code for Mac2.mega-flow.spec.tswas written specifically against Mac2's limitations (deep links + mock log + RPC, no UI assertions) because Mac2 couldn't see the DOM at all. The asymmetry made macOS coverage shallow and any unified test ergonomics impossible.Additionally, each spec invoked
e2e-run-spec.sh, which started a fresh Appium / tauri-driver and a freshOpenHuman.appper spec — wall-clock cost dominated by setup, not assertions.Solution
CEF (the runtime under Tauri here) already exposes a Chrome DevTools Protocol port on
127.0.0.1:19222on every platform (enabled inapp/src-tauri/src/lib.rs:1556). That makes a unified driver possible:wdio.conf.ts: single Chromium capability withgoog:chromeOptions.debuggerAddress: 127.0.0.1:19222. Same caps on macOS / Linux / Windows.maxInstances: 1keeps everything in one session. Abefore:hook switches to the main app webview after attach (defensive; the runner already prunes the prewarm target).scripts/e2e-run-session.sh(new): launches the built CEF binary once, waits for CDP, prunes the prewarmabout:blanktarget via/json/close/<id>so chromedriver doesn't attach to it (would otherwise die mid-run when CEF GC'd that target), parses Chromium version from/json/versionand downloads a matching chromedriver from Chrome for Testing, then starts Appium with thechromiumdriver and runs WDIO.scripts/e2e-build.sh: now invokespnpm tauri:ensureand usescargo tauri build(notpnpm exec tauri build) so the .app ships the CEF framework. Without this, the bundle panics at startup insidecef::library_loader::LibraryLoader::new(the failure mode CLAUDE.md warns about).scripts/e2e-run-spec.sh: thin shim thatexecse2e-run-session.sh, so existing per-spec scripts (test:e2e:cron-jobs, etc.) keep working.helpers/platform.ts: collapsed —isTauriDriver()returnstrue,isMac2()returnsfalse,supportsExecuteScript()returnstrueon every platform. The ~40 existing specs that branched on these uniformly take the DOM-capable code path, which Chromium driver supports everywhere.helpers/core-rpc.ts: drops the WebView path that didawait import('@tauri-apps/api/core')insidebrowser.execute. That dynamic specifier only resolved under tauri-driver's renderer import map; under direct CDP it 404s. The Node-side HTTP path (port probe + bearer token from${tmpdir}/openhuman-e2e-rpc-token) is driver-agnostic and is now the only path.helpers/app-helpers.ts:waitForAppdoes a real readiness check.This is phase 1 of a multi-phase migration. The unified driver works end-to-end (smoke spec green on macOS). Phase 2 work that is not in this PR: the mega-flow and other deep-link-heavy specs hit a real CEF interaction — the vendored CEF runtime drops and recreates the main
WebviewWindowon certain events (seeapp/src-tauri/src/lib.rs:907), which terminates the chromedriver session bound to the original target ID. That needs either session-reattach logic in the test helpers or a behavior change in the CEF window-recreation path; the right shape isn't obvious yet and warrants its own PR.Submission Checklist
Impact
isTauriDriver()will execute their DOM assertions on macOS for the first time.pnpm test:e2e:cron-jobsetc. continue to work (legacy runner is a shim). New entry point ispnpm test:e2e:session[:full].tauri-driverand rane2e-run-spec.shper spec can be simplified to a single session run; that simplification is deliberately out of scope here so the foundation can be validated first.Related
mega-flow.spec.tsand other deep-link-heavy specs run green under the unified driver.helpers/element-helpers.ts(Mac2 XPath paths are now unreachable).specs/legacy/once the mega-orchestrator covers their flows so the defaulttest:e2e:sessionis fast.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
e2e/unified-appium-chromiumValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm test:e2e:session test/e2e/specs/smoke.spec.ts— 3/3 passing on macOS (arm64) against CEF Chromium 146.0.7680.165Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
pnpm test:e2e:cron-jobs, etc.) keep working via a shim that delegates to the new session runner.helpers/platform.tsshimsisTauriDriver()/isMac2()/supportsExecuteScript()to their now-unified values so the 40+ specs that branched on these uniformly take the DOM path without per-spec edits.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Tests
Chores