Respect command-specific timeouts in replay

replay.sh hard-caps every agent-browser command to 10 seconds via timeout "$CMD_TIMEOUT" ..., so recorded steps that intentionally request longer waits (for example wait --timeout 60000/120000 in the new extension recordings) will still be killed at 10s. This makes long-running LLM/tool steps fail even when the recording specifies a larger timeout, causing deterministic false failures in the new E2E workflow.

Useful? React with 👍 / 👎.

fixed

Return the actual affirmative dialog response in E2E mode

In ENABLE_PLAYWRIGHT mode this handler always returns { response: 1 }, but several callers treat index 0 as the confirm action (e.g., KeyboardShortcutsSection conflict/reset dialogs). That means these dialogs are auto-cancelled instead of auto-confirmed during E2E runs, so test behavior diverges from user behavior and some flows become untestable.

Useful? React with 👍 / 👎.

The current E2E tests (eg: delete recipe) that trigger showMessageBox all use ['Cancel', 'Action'] button order, so response: 1 correctly auto-confirms them. The keyboard shortcut dialogs that use index 0 as confirm aren't used in any E2E test today. We'll revisit if that changes.

Reserve CDP ports atomically across parallel workers

pick_port does a check-then-use (lsof then echo) on a shared 9300–9399 range, but e2e-run-all.sh launches multiple tests concurrently (xargs -P "$WORKERS"). Two workers can select the same still-unbound port, which makes one session connect to the wrong Electron instance or fail nondeterministically once one process binds first. This creates cross-test contamination and flaky CI outcomes in the new parallel runner.

Useful? React with 👍 / 👎.

addressed this

Always stop the test session on startup timeout

When app startup does not become ready within 30s, run_one returns early before reaching the teardown call (e2e-stop.sh). Since the app was launched in a detached screen session, this path can leave orphaned Electron/goosed processes and stale session artifacts, which then interfere with subsequent recordings in the same run (e.g., port conflicts and residual state).

Useful? React with 👍 / 👎.

fixed

Stop goosed cleanly instead of SIGKILLing Electron

This teardown path force-kills the CDP listener with kill -9, which bypasses Electron shutdown hooks that normally call goosedResult.cleanup() in main.ts. Because goosed is spawned as goosed agent (no session path in argv), the later pattern kill in e2e-stop.sh does not reliably target it, so tests can leave orphan backend processes running and contaminate subsequent runs.

Useful? React with 👍 / 👎.

The primary shutdown is screen -X quit now, which sends SIGHUP and lets Electron run its shutdown hooks including goosedResult.cleanup(). The kill -9 lines after that are safety nets that only fire on processes that survived the graceful shutdown.

Gate parallel workers when single-instance lock applies

The runner always executes recordings in parallel, but the app still enforces app.requestSingleInstanceLock() on non-macOS in main.ts. On Linux/Windows, only one Electron instance can remain alive, so additional workers will fail startup and be reported as test failures; this needs a platform guard (or forcing --workers 1) to avoid deterministic false negatives outside macOS.

Useful? React with 👍 / 👎.

we only supports macos at the moment

Respect scenario time budgets before test-level timeout

Each recording is wrapped in a fixed timeout "$TIMEOUT" (default 120s), but new recordings include waits up to 120s after earlier long waits (for example, extension-install-playwright.batch.json has 60s + 60s + 120s waits). This causes the harness to terminate otherwise valid long-running scenarios before their own declared timeouts elapse, producing avoidable TIMEOUT failures.

Useful? React with 👍 / 👎.

removed this timeout as we already have timeout when running agent browser command

Preserve requested chat directory over env default

createChat now always prefers process.env.GOOSE_WORKING_DIR over the per-window dir argument, so any flow that passes a specific directory (e.g., deep links/new-window creation with dir) is silently ignored whenever that env var is set. This regresses directory-scoped behavior in those launches: users/tests can request a different folder, but startGoosed still starts in the env folder. Restricting the env override to initial defaulting (or to explicit E2E mode) avoids breaking dir-driven window creation.

Useful? React with 👍 / 👎.

It is fine now. GOOSE_WORKING_DIR never worked before this PR. This change is specifically for e2e test isolation. Will create a follow-up PR to consolidate the existing duplicated openDir logic so directory resolution is handled in one place.

Use ERE alternation when killing agent-browser helpers

pkill patterns are interpreted as regular expressions (BSD/FreeBSD pgrep/pkill docs describe these as extended regex), so alternation should be |, not \|. With the current pattern, this command matches a literal pipe and does not match agent-browser-darwin or agent-browser-linux, leaving helper processes alive after teardown; those leftovers can interfere with later E2E runs on the same machine.

Useful? React with 👍 / 👎.

Kill goosed explicitly for per-session teardown

This teardown path does not reliably terminate the backend process after force-killing Electron. stop_session only pattern-kills by "$SESSION_DIR", but startGoosed launches goosed as goosed agent with the session path only in cwd (not argv), so that match can miss; and in the normal runner path we call e2e-stop.sh "$TEST_NAME", which exits before the global fallback sweeps at the bottom of the script. Fresh evidence: ui/desktop/src/goosed.ts uses spawnArgs = ['agent'] and cwd: workingDir, so argv-based pkill -f "$SESSION_DIR" is not a reliable cleanup mechanism. When this miss happens, orphan goosed processes can leak into later tests and cause cross-test contamination/flakes.

Useful? React with 👍 / 👎.

screen -X quit is the primary kill — it triggers Electron's graceful shutdown which SIGTERMs goosed. The pkill lines are last-resort fallbacks. We also fixed a screen session name mismatch that was preventing the graceful path from working.

Make timeout available on macOS before running E2E

On macos-latest, this step only runs brew install coreutils when timeout is missing, but the replay harness later calls timeout directly for every command. Homebrew coreutils installs gtimeout unless the gnubin path is added, so in the common case where timeout is absent the E2E run can still fail with timeout: command not found before tests execute. Please either export the coreutils gnubin directory (or symlink) here, or update the scripts to fall back to gtimeout.

Useful? React with 👍 / 👎.

timeout wasn't available on the macOS runner by default, so this step installs coreutils as a fallback. Once installed, timeout is available on PATH and the scripts work. This has been verified in CI.

Drop nonexistent unzip step after gh artifact download

When results_url is provided, these instructions call gh run download ... --dir /tmp/e2e-results and then immediately unzip /tmp/e2e-results.zip. gh run download already extracts artifacts into the target directory, so this zip file is not produced; the extra unzip command will fail and block Step 1 for CI-artifact debugging runs.

Useful? React with 👍 / 👎.

Check lock age before breaking startup lock

The stale-lock logic removes STARTUP_LOCK after a waiter has blocked for 60 seconds, but that does not prove the lock holder crashed. If a legitimate startup takes longer than 60s (e.g., slow electron-forge rebuild), another worker will delete the active lock and start concurrently, reintroducing the .vite/build race this lock is intended to prevent and causing intermittent startup/test failures.

Useful? React with 👍 / 👎.

Startup usually takes less than 60s. stale lock blocking CI forever might be worse than a rare race condition.

Hold startup lock until timed-out app is fully stopped

On the startup-timeout path, the script releases STARTUP_LOCK before calling e2e-stop.sh. If the timed-out electron-forge process is still rebuilding .vite/build when the 30s wait expires, another worker can immediately acquire the lock and start a second app concurrently, reintroducing the same shared-build race this lock is meant to prevent. Keep the lock held until teardown of the timed-out session completes.

Useful? React with 👍 / 👎.

fixed

Run global fallback cleanup even without session dirs

This early exit 0 skips the global fallback pkill cleanup at the bottom of the script whenever /tmp/goose-e2e/sessions is absent. In the normal runner flow, per-test teardown removes session directories, so a later all-sessions stop can bypass fallback cleanup and leave orphan helper processes alive from previous failures, which can interfere with subsequent runs. The script should skip only the per-session loop, not terminate before global cleanup.

Useful? React with 👍 / 👎.

fixed

Break startup lock only once after stale detection

Fresh evidence: because waited is never reset, once a worker has waited 60s it runs rm -rf "$STARTUP_LOCK" on every 0.5s retry. If another worker is legitimately still starting Electron, this loop can repeatedly delete an active lock and let multiple startups run concurrently, which reintroduces the shared .vite/build race this lock is meant to prevent. Limit stale-lock stealing to a single verified stale lock (e.g., based on lock age/owner) before retrying.

Useful? React with 👍 / 👎.

you put the same comment as above. I have replied with

"Startup usually takes less than 60s. stale lock blocking CI forever might be worse than a rare race condition."

Pin Playwright MCP dependency to a fixed version

This recording installs Playwright MCP via @latest, so each CI run can pull different upstream code and silently change behavior or break setup even when this repo is unchanged. That makes the new E2E suite non-reproducible and can create false failures unrelated to the PR under test; please pin a concrete package version in the deeplink command.

Useful? React with 👍 / 👎.

Avoid live Google dependency in deterministic replay

This step drives a real google.com search and then asserts success within a short timeout, which makes the test depend on external network availability, consent/CAPTCHA behavior, and third-party page changes. In CI this can fail even when Goose itself is healthy, so the scenario should target a controlled fixture/local page (or mocked endpoint) to keep replay deterministic.

Useful? React with 👍 / 👎.