diff --git a/backend/scripts/agent-screenshot.sh b/backend/scripts/agent-screenshot.sh index 830825267..f4002e7ef 100755 --- a/backend/scripts/agent-screenshot.sh +++ b/backend/scripts/agent-screenshot.sh @@ -43,12 +43,52 @@ set -euo pipefail # process-level `timeout` utility instead, which preserves daemon identity and # lets the caller inspect/close the same session after this helper returns. +BROWSER_ERROR_FILE="" +WARMUP_OUT="" +CAPTURE_OUT="" + +cleanup() { + local path + for path in "$BROWSER_ERROR_FILE" "$WARMUP_OUT" "$CAPTURE_OUT"; do + [ -z "$path" ] || rm -f "$path" + done +} + +die() { + printf 'agent-screenshot.sh: %s\n' "$*" >&2 + if [ -n "$BROWSER_ERROR_FILE" ] && [ -s "$BROWSER_ERROR_FILE" ]; then + tail -n 2 "$BROWSER_ERROR_FILE" | sed 's/^/agent-browser: /' >&2 + fi + exit 1 +} + +browser_command() { + local timeout_seconds="$1" + local status=0 + shift + : > "$BROWSER_ERROR_FILE" + timeout "${timeout_seconds}s" agent-browser "$@" \ + 2>"$BROWSER_ERROR_FILE" || status=$? + if [ "$status" -eq 124 ] && [ ! -s "$BROWSER_ERROR_FILE" ]; then + printf 'command timed out after %ss\n' "$timeout_seconds" \ + > "$BROWSER_ERROR_FILE" + fi + return "$status" +} + +browser_wait() { + local status=0 + : > "$BROWSER_ERROR_FILE" + agent-browser wait "$@" 2>"$BROWSER_ERROR_FILE" || status=$? + return "$status" +} + browser_eval_retry() { local expression="$1" local output="" local attempt for attempt in 1 2 3; do - if output="$(timeout 5s agent-browser eval "$expression" 2>/dev/null)"; then + if output="$(browser_command 5 eval "$expression")"; then printf '%s' "$output" return 0 fi @@ -65,10 +105,10 @@ browser_screenshot_retry() { # Navigation/service-worker handoffs can replace the page after an earlier # viewport command. Configure the page that will produce THIS screenshot, # not merely the page that existed at helper startup. - if ! prepare_capture_viewport; then + if ! browser_set_viewport_retry; then continue fi - if timeout 5s agent-browser screenshot "$output_path" >/dev/null 2>&1; then + if browser_command 5 screenshot "$output_path" >/dev/null; then byte_count="$(wc -c < "$output_path" 2>/dev/null || printf '0')" if { [ "${CAPTURE_MIN_BYTES:-0}" -le 0 ] \ || [ "${byte_count:-0}" -ge "${CAPTURE_MIN_BYTES}" ]; } \ @@ -110,14 +150,14 @@ browser_open_origin_retry() { local settled="" local attempt for attempt in 1 2 3 4 5; do - timeout 5s agent-browser open "$url" >/dev/null 2>&1 || true + browser_command 5 open "$url" >/dev/null || true sleep 0.2 current="$( - timeout 5s agent-browser get url 2>/dev/null || true + browser_command 5 get url || true )" sleep 0.2 confirmed="$( - timeout 5s agent-browser get url 2>/dev/null || true + browser_command 5 get url || true )" if [ "$current" != "$confirmed" ]; then # In-shell deep links intentionally canonicalize to `/shell/` after the @@ -132,7 +172,7 @@ browser_open_origin_retry() { esac sleep 0.2 settled="$( - timeout 5s agent-browser get url 2>/dev/null || true + browser_command 5 get url || true )" [ "$confirmed" = "$settled" ] && return 0 ;; @@ -158,14 +198,14 @@ browser_open_exact_retry() { local confirmed="" local attempt for attempt in 1 2 3 4 5; do - timeout 5s agent-browser open "$url" >/dev/null 2>&1 || true + browser_command 5 open "$url" >/dev/null || true sleep 0.2 current="$( - timeout 5s agent-browser get url 2>/dev/null || true + browser_command 5 get url || true )" sleep 0.2 confirmed="$( - timeout 5s agent-browser get url 2>/dev/null || true + browser_command 5 get url || true )" [ "$current" = "$url" ] && [ "$confirmed" = "$url" ] && return 0 done @@ -175,8 +215,8 @@ browser_open_exact_retry() { browser_set_viewport_retry() { local attempt for attempt in 1 2 3 4 5; do - if timeout 5s agent-browser set viewport "$VIEWPORT_WIDTH" "$VIEWPORT_HEIGHT" \ - >/dev/null 2>&1; then + if browser_command 5 set viewport "$VIEWPORT_WIDTH" "$VIEWPORT_HEIGHT" \ + >/dev/null; then return 0 fi sleep 0.2 @@ -244,7 +284,7 @@ ROUTE="${1:-}" OUT="${2:-}" if [ -z "$ROUTE" ]; then - echo "agent-screenshot.sh: route required" >&2 + printf '%s\n' "agent-screenshot.sh: route required" >&2 echo "Usage: agent-screenshot.sh [--content-only] [--preserve-cache] [out.png]" >&2 exit 1 fi @@ -254,8 +294,7 @@ fi # written elsewhere (e.g. /tmp) is viewable by the agent but 404s if embedded. if [ -z "$OUT" ]; then if [ -z "${CHAT_ID:-}" ]; then - echo "agent-screenshot.sh: no out.png given and CHAT_ID unset" >&2 - exit 1 + die "no out.png given and CHAT_ID unset" fi OUT="/data/chats/${CHAT_ID}/media/shot-$(date +%s%N).png" fi @@ -269,13 +308,11 @@ esac mkdir -p "$(dirname "$OUT")" if [ -z "${AGENT_TOKEN:-}" ] || [ -z "${API_BASE_URL:-}" ]; then - echo "agent-screenshot.sh: AGENT_TOKEN and API_BASE_URL must be set" >&2 - exit 1 + die "AGENT_TOKEN and API_BASE_URL must be set" fi if ! command -v agent-browser >/dev/null 2>&1; then - echo "agent-screenshot.sh: agent-browser not on PATH" >&2 - exit 1 + die "agent-browser not on PATH" fi # Prefer the runner-provided per-chat session/profile. Fall back from CHAT_ID @@ -296,8 +333,7 @@ fi # they see. chat.py exports VIEWPORT_WIDTH/HEIGHT from the React # shell's per-turn payload; screenshots require those values. if [ -z "${VIEWPORT_WIDTH:-}" ] || [ -z "${VIEWPORT_HEIGHT:-}" ]; then - echo "agent-screenshot.sh: VIEWPORT_WIDTH and VIEWPORT_HEIGHT must be set" >&2 - exit 1 + die "VIEWPORT_WIDTH and VIEWPORT_HEIGHT must be set" fi # Existing agent sessions keep the env snapshot they started with, and manual # callers can bypass chat.py entirely. Normalize again at this executable @@ -322,20 +358,32 @@ if not all(math.isfinite(value) and value > 0 for value in values): print(*(max(1, round(value)) for value in values)) PY )"; then - echo "agent-screenshot.sh: VIEWPORT_WIDTH and VIEWPORT_HEIGHT must be positive numbers" >&2 - exit 1 + die "VIEWPORT_WIDTH and VIEWPORT_HEIGHT must be positive numbers" fi read -r VIEWPORT_WIDTH VIEWPORT_HEIGHT <<<"$NORMALIZED_VIEWPORT" +# One chat can host parallel agents, but its browser commands target one +# persistent profile. Serialize the complete navigation/capture transaction so +# two helpers cannot interleave routes, viewport changes, or output ownership. +if [ -n "${AGENT_BROWSER_PROFILE:-}" ]; then + if ! command -v flock >/dev/null 2>&1; then + die "flock is required for shared browser profiles" + fi + exec 9>"${AGENT_BROWSER_PROFILE}.capture.lock" + if ! flock -w 30 9; then + die "another capture still owns this browser profile" + fi +fi + clear_stale_browser_profile_lock +BROWSER_ERROR_FILE="$(mktemp "${TMPDIR:-/tmp}/mobius-agent-browser-error.XXXXXX")" +trap cleanup EXIT # Start the browser, then wait narrowly for its command socket before applying # viewport state. A cold launch can return before that socket is connectable. -timeout 5s agent-browser open "${API_BASE_URL}/api/browser-bootstrap" \ - >/dev/null 2>&1 || true +browser_command 5 open "${API_BASE_URL}/api/browser-bootstrap" >/dev/null || true if ! browser_set_viewport_retry; then - echo "agent-screenshot.sh: browser did not become ready for viewport configuration" >&2 - exit 1 + die "browser did not become ready for viewport configuration" fi # A retained test profile can start under an older service worker that handles @@ -351,8 +399,7 @@ if [ "$PRESERVE_CACHE" -eq 0 ]; then "(async () => { try { const regs = await navigator.serviceWorker?.getRegistrations?.() || []; await Promise.all(regs.map((r) => r.unregister())); } catch {} return true })()" \ >/dev/null || true if ! browser_open_exact_retry "about:blank"; then - echo "agent-screenshot.sh: browser did not detach its stale bootstrap page" >&2 - exit 1 + die "browser did not detach its stale bootstrap page" fi fi @@ -386,14 +433,13 @@ print( + visual + reset + " return true })()" ) ' \ - | timeout 5s agent-browser eval --stdin >/dev/null 2>&1; then + | browser_command 5 eval --stdin >/dev/null; then TOKEN_READY=1 break fi done if [ "$TOKEN_READY" -ne 1 ]; then - echo "agent-screenshot.sh: browser origin did not remain ready for authentication" >&2 - exit 1 + die "browser origin did not remain ready for authentication" fi # A per-chat Chromium profile deliberately survives browser close, which is @@ -407,8 +453,7 @@ fi TARGET_ROUTE="$ROUTE" if [ "$PRESERVE_CACHE" -eq 0 ]; then if ! browser_open_exact_retry "about:blank"; then - echo "agent-screenshot.sh: browser did not detach before target navigation" >&2 - exit 1 + die "browser did not detach before target navigation" fi CAPTURE_NONCE="$(date +%s%N)" case "$TARGET_ROUTE" in @@ -419,19 +464,17 @@ fi # Now navigate to the actual target route, authenticated. if ! browser_open_origin_retry "${API_BASE_URL}${TARGET_ROUTE}" target; then - echo "agent-screenshot.sh: browser did not reach the target route" >&2 - exit 1 + die "browser did not reach the target route" fi # The URL can canonicalize before the replacement document has committed. # Applying device metrics during that gap reports success on the outgoing page, # then the newly-created shell page falls back to Chromium's default viewport. # Wait on browser paint ownership before configuring the final page. -if ! agent-browser wait --fn \ +if ! browser_wait --fn \ "document.readyState === 'complete' && performance.getEntriesByName('first-contentful-paint').length > 0" \ - >/dev/null 2>&1; then - echo "agent-screenshot.sh: target document did not finish its initial paint" >&2 - exit 1 + >/dev/null; then + die "target document did not finish its initial paint" fi # Let the navigation commit without asking the renderer to poll the transcript. @@ -442,7 +485,7 @@ sleep 0.3 # Dismiss the PWA install banner if it surfaces — it covers the bottom # of the view and would distract from the actual page. -timeout 2s agent-browser find text "Not now" click >/dev/null 2>&1 || true +browser_command 2 find text "Not now" click >/dev/null || true sleep 0.3 # Token presence alone is not proof of authentication: App mounts Shell from @@ -454,8 +497,7 @@ AUTH_OK="$(browser_eval_retry \ "(async () => { const token = localStorage.getItem('token'); if (!token || document.querySelector('input[type=password]')) return false; try { const res = await fetch('/api/chats?agent-screenshot-auth=' + Date.now(), { cache: 'no-store', headers: { Authorization: 'Bearer ' + token } }); return res.status === 200 && !!localStorage.getItem('token') && !document.querySelector('input[type=password]'); } catch { return false; } })()" \ || true)" if [ "$AUTH_OK" != "true" ]; then - echo "agent-screenshot.sh: authentication failed; the token was rejected or the login page remained visible" >&2 - exit 1 + die "authentication failed; the token was rejected or the login page remained visible" fi # For shell routes, prove the browser loaded the same hashed entry asset that @@ -475,8 +517,7 @@ case "$ROUTE" in if [ "$PRESERVE_CACHE" -eq 0 ]; then DIST_INDEX="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../frontend" && pwd)/dist/index.html" if [ ! -f "$DIST_INDEX" ]; then - echo "agent-screenshot.sh: current frontend build not found at $DIST_INDEX" >&2 - exit 1 + die "current frontend build not found at $DIST_INDEX" fi CURRENT_SHELL_ENTRY="$( python3 - "$DIST_INDEX" <<'PY' @@ -491,8 +532,7 @@ if not match: print(match.group(1).rsplit("/", 1)[-1]) PY )" || { - echo "agent-screenshot.sh: current shell entry asset could not be resolved" >&2 - exit 1 + die "current shell entry asset could not be resolved" } LOADED_SHELL_ENTRY_RAW="$( browser_eval_retry \ @@ -505,8 +545,7 @@ PY 2>/dev/null || printf '%s' "$LOADED_SHELL_ENTRY_RAW" )" if [ "$LOADED_SHELL_ENTRY" != "$CURRENT_SHELL_ENTRY" ]; then - echo "agent-screenshot.sh: stale shell loaded (expected $CURRENT_SHELL_ENTRY, got ${LOADED_SHELL_ENTRY:-none})" >&2 - exit 1 + die "stale shell loaded (expected $CURRENT_SHELL_ENTRY, got ${LOADED_SHELL_ENTRY:-none})" fi fi @@ -516,33 +555,23 @@ PY # its private handoff classes or compositor attributes. Once the owner says # settled, give style/layout two frames to commit. SHELL_SETTLED_EXPR="document.querySelector('.shell[data-workspace-visual-state=\"settled\"]') !== null && performance.getEntriesByName('first-contentful-paint').length > 0" - if ! agent-browser wait --fn "$SHELL_SETTLED_EXPR" >/dev/null 2>&1; then - echo "agent-screenshot.sh: shell did not reach a settled visual state before capture" >&2 - exit 1 + if ! browser_wait --fn "$SHELL_SETTLED_EXPR" >/dev/null; then + die "shell did not reach a settled visual state before capture" fi if ! browser_eval_retry \ "new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(() => resolve(true))))" \ >/dev/null; then - echo "agent-screenshot.sh: shell did not commit its settled frame before capture" >&2 - exit 1 + die "shell did not commit its settled frame before capture" fi ;; esac # Device metrics attach to one CDP page. A canonical redirect, a service-worker # handoff, or agent-browser's about:blank detach can replace that page after the -# startup viewport command. Reapply immediately before each capture attempt; -# browser_screenshot_retry validates the kept PNG's exact IHDR dimensions. -prepare_capture_viewport() { - # Do not insert a separate renderer round-trip here. A document replacement - # can land between configuration and that probe; the kept PNG's IHDR below is - # the exact, race-free proof of which viewport actually produced evidence. - browser_set_viewport_retry -} - -if ! prepare_capture_viewport; then - echo "agent-screenshot.sh: target page did not retain the requested viewport" >&2 - exit 1 +# startup viewport command. Reapply before drawer/app checks and immediately +# before every capture attempt; the kept PNG's IHDR is the race-free proof. +if ! browser_set_viewport_retry; then + die "target page did not retain the requested viewport" fi # A fresh phone-width shell can restore with the modal navigation drawer open @@ -553,11 +582,10 @@ browser_eval_retry \ "(() => { const b = document.querySelector('button[aria-label=\"Toggle navigation\"][aria-expanded=\"true\"]'); if (window.innerWidth < 768 && b) b.click(); return true })()" \ >/dev/null || true if [ "$VIEWPORT_WIDTH" -lt 768 ]; then - if ! agent-browser wait --fn \ + if ! browser_wait --fn \ "!document.querySelector('.drawer-overlay--blocking') && !document.querySelector('.drawer:not(.drawer--persistent).drawer--open')" \ - >/dev/null 2>&1; then - echo "agent-screenshot.sh: mobile navigation did not finish closing before capture" >&2 - exit 1 + >/dev/null; then + die "mobile navigation did not finish closing before capture" fi fi @@ -568,13 +596,17 @@ fi # loading skeleton. Keep the predicate as a simple boolean expression — # agent-browser's wait parser has timed out on equivalent IIFE forms. case "$ROUTE" in - /app/[0-9]*) + /app/*) APP_ID="${ROUTE#/app/}" APP_ID="${APP_ID%%[/?#]*}" + case "$APP_ID" in + ''|*[!0-9]*) + die "in-shell app routes require a numeric app id" + ;; + esac READY_EXPR="document.querySelector('iframe[data-app-id=\"${APP_ID}\"]') !== null && document.querySelector('iframe[data-app-id=\"${APP_ID}\"]')?.parentElement.querySelector('.canvas-loading') === null" - if ! agent-browser wait --fn "$READY_EXPR" >/dev/null 2>&1; then - echo "agent-screenshot.sh: app ${APP_ID} did not reach its mounted frame before capture" >&2 - exit 1 + if ! browser_wait --fn "$READY_EXPR" >/dev/null; then + die "app ${APP_ID} did not reach its mounted frame before capture" fi ;; esac @@ -586,24 +618,26 @@ esac # wait two frames before keeping evidence. This is a renderer handshake, not a # guessed sleep, and the temporary image never enters chat media. WARMUP_OUT="$(mktemp "${TMPDIR:-/tmp}/mobius-screenshot-warmup.XXXXXX.png")" -trap 'rm -f "$WARMUP_OUT"' EXIT +# Validate the final frame before publishing it. Wrappers intentionally reuse +# friendly paths such as shell.png and app-42.png; a failed capture must never +# replace the last known-good image with a partial or misleading frame. +CAPTURE_OUT="$(mktemp "$(dirname "$OUT")/.mobius-screenshot.XXXXXX.png")" if ! browser_screenshot_retry "$WARMUP_OUT"; then - echo "agent-screenshot.sh: page remained too busy to prime capture" >&2 - exit 1 + die "page remained too busy to prime capture" fi if ! browser_eval_retry \ "new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(() => resolve(true))))" \ >/dev/null; then - echo "agent-screenshot.sh: page did not commit after capture priming" >&2 - exit 1 + die "page did not commit after capture priming" fi -if ! browser_screenshot_retry "$OUT"; then - echo "agent-screenshot.sh: page remained too busy to capture after bounded retries" >&2 - exit 1 +if ! browser_screenshot_retry "$CAPTURE_OUT"; then + die "page remained too busy to capture after bounded retries" fi +mv -f "$CAPTURE_OUT" "$OUT" +CAPTURE_OUT="" rm -f "$WARMUP_OUT" -trap - EXIT +WARMUP_OUT="" echo "${OUT}" # Also print the ready-to-paste chat embed. The partner sees ONLY embedded diff --git a/backend/tests/test_agent_screenshot_auth.py b/backend/tests/test_agent_screenshot_auth.py index f174276a8..f5ebee6b2 100644 --- a/backend/tests/test_agent_screenshot_auth.py +++ b/backend/tests/test_agent_screenshot_auth.py @@ -1,11 +1,14 @@ """Regression coverage for authenticated screenshot readiness checks.""" +import fcntl from pathlib import Path import os import shutil import socket import subprocess +import pytest + SCRIPT = Path(__file__).parents[1] / "scripts" / "agent-screenshot.sh" PREVIEW_APP = Path(__file__).parents[1] / "scripts" / "preview_app.sh" @@ -36,6 +39,9 @@ def _fixture_script(tmp_path: Path) -> Path: def _fake_browser(tmp_path: Path) -> tuple[Path, Path]: marker = tmp_path / "screenshot-called" + sleep = tmp_path / "sleep" + sleep.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + sleep.chmod(0o755) png_writer = tmp_path / "fake-png.py" png_writer.write_text( "import struct, sys\n" @@ -50,6 +56,10 @@ def _fake_browser(tmp_path: Path) -> tuple[Path, Path]: browser.write_text( "#!/bin/sh\n" "printf '%s\\n' \"$*\" >> \"$FAKE_BROWSER_LOG\"\n" + "printf '%s|%s|%s|%s\\n' " + "\"${AGENT_BROWSER_SESSION-}\" \"${AGENT_BROWSER_PROFILE-}\" " + "\"${AGENT_BROWSER_ARGS-}\" \"${AGENT_BROWSER_DEFAULT_TIMEOUT-}\" " + ">> \"$FAKE_BROWSER_IDENTITY_LOG\"\n" "case \"$1\" in\n" " open)\n" " printf '%s\\n' \"$2\" > \"$FAKE_BROWSER_URL_FILE\"\n" @@ -81,7 +91,20 @@ def _fake_browser(tmp_path: Path) -> tuple[Path, Path]: " exit 1\n" " fi\n" " ;;\n" + " wait)\n" + " if [ \"${FAKE_WAIT_ERROR:-0}\" = 1 ]; then\n" + " printf '%s\\n' 'renderer disconnected' >&2\n" + " exit 1\n" + " fi\n" + " ;;\n" " screenshot)\n" + " count=$(cat \"$FAKE_SCREENSHOT_COUNT_FILE\" 2>/dev/null || printf 0)\n" + " count=$((count + 1))\n" + " printf '%s\\n' \"$count\" > \"$FAKE_SCREENSHOT_COUNT_FILE\"\n" + " if [ \"${FAKE_SCREENSHOT_FAIL_AFTER_WARMUP:-0}\" = 1 ] && [ \"$count\" -gt 1 ]; then\n" + " printf partial > \"$2\"\n" + " exit 1\n" + " fi\n" " if [ \"${FAKE_SCREENSHOT_FAIL_ONCE:-0}\" = 1 ] && [ ! -e \"$FAKE_SCREENSHOT_RETRY_MARKER\" ]; then\n" " : > \"$FAKE_SCREENSHOT_RETRY_MARKER\"\n" " exit 1\n" @@ -111,8 +134,13 @@ def _run_helper( loaded_asset: str | None = None, viewport_fail_once: bool = False, screenshot_fail_once: bool = False, + screenshot_fail_after_warmup: bool = False, canonical_target_url: str | None = None, screenshot_tiny_once: bool = False, + wait_error: bool = False, + existing_output: bytes | None = None, + profile_locked: bool = False, + subprocess_timeout: float | None = None, profile_lock_target: str | None = None, profile_lock_artifacts: tuple[str, ...] = ( "SingletonLock", "SingletonCookie", "SingletonSocket", @@ -124,24 +152,35 @@ def _run_helper( browser_log = tmp_path / "browser.log" browser_profile = tmp_path / "browser-profile" browser_profile.mkdir() + if existing_output is not None: + output.write_bytes(existing_output) if profile_lock_target is not None: for artifact in profile_lock_artifacts: (browser_profile / artifact).symlink_to(profile_lock_target) env = { **os.environ, "PATH": f"{tmp_path}:{os.environ['PATH']}", + "TMPDIR": str(tmp_path), "AGENT_TOKEN": "test-token", "API_BASE_URL": "http://mobius.test", "VIEWPORT_WIDTH": str(viewport_width), "VIEWPORT_HEIGHT": str(viewport_height), + "AGENT_BROWSER_SESSION": "test-session", "AGENT_BROWSER_PROFILE": str(browser_profile), + "AGENT_BROWSER_ARGS": "--test-daemon-identity", + "AGENT_BROWSER_DEFAULT_TIMEOUT": "", "FAKE_AUTH_OK": "true" if auth_ok else "false", "FAKE_LOADED_ASSET": loaded_asset or SHELL_ENTRY, "FAKE_BROWSER_LOG": str(browser_log), + "FAKE_BROWSER_IDENTITY_LOG": str(tmp_path / "browser-identity.log"), "FAKE_SCREENSHOT_MARKER": str(marker), "FAKE_VIEWPORT_FAIL_ONCE": "1" if viewport_fail_once else "0", "FAKE_VIEWPORT_MARKER": str(tmp_path / "viewport-ready"), "FAKE_SCREENSHOT_FAIL_ONCE": "1" if screenshot_fail_once else "0", + "FAKE_SCREENSHOT_FAIL_AFTER_WARMUP": ( + "1" if screenshot_fail_after_warmup else "0" + ), + "FAKE_SCREENSHOT_COUNT_FILE": str(tmp_path / "screenshot-count"), "FAKE_SCREENSHOT_RETRY_MARKER": str(tmp_path / "screenshot-retried"), "FAKE_SCREENSHOT_TINY_ONCE": "1" if screenshot_tiny_once else "0", "FAKE_SCREENSHOT_TINY_MARKER": str(tmp_path / "screenshot-tiny"), @@ -150,6 +189,7 @@ def _run_helper( "FAKE_BROWSER_NEXT_URL_FILE": str(tmp_path / "browser-next-url"), "FAKE_CANONICAL_TARGET_URL": canonical_target_url or "", "FAKE_BROWSER_STDIN_LOG": str(tmp_path / "browser-stdin.log"), + "FAKE_WAIT_ERROR": "1" if wait_error else "0", } args = ["bash", str(script)] if content_only: @@ -157,13 +197,23 @@ def _run_helper( if preserve_cache: args.append("--preserve-cache") args.extend([route, str(output)]) - result = subprocess.run( - args, - env=env, - text=True, - capture_output=True, - check=False, - ) + lock_handle = None + try: + if profile_locked: + lock_handle = Path(f"{browser_profile}.capture.lock").open("w") + fcntl.flock(lock_handle, fcntl.LOCK_EX) + result = subprocess.run( + args, + env=env, + text=True, + capture_output=True, + check=False, + timeout=subprocess_timeout, + ) + finally: + if lock_handle is not None: + fcntl.flock(lock_handle, fcntl.LOCK_UN) + lock_handle.close() return result, output, marker, browser_log @@ -435,10 +485,32 @@ def test_cold_capture_opens_browser_before_configuring_viewport(tmp_path: Path): assert "open http://mobius.test/" not in commands -def test_browser_commands_never_split_daemons_with_per_call_environment(): - source = SCRIPT.read_text(encoding="utf-8") - assert "AGENT_BROWSER_DEFAULT_TIMEOUT=" not in source - assert "timeout 5s agent-browser" in source +def test_browser_commands_keep_one_daemon_identity(tmp_path: Path): + result, _, _, _ = _run_helper(tmp_path, auth_ok=True) + + assert result.returncode == 0, result.stderr + identities = (tmp_path / "browser-identity.log").read_text( + encoding="utf-8", + ).splitlines() + expected = ( + f"test-session|{tmp_path / 'browser-profile'}|--test-daemon-identity|" + ) + assert identities + assert set(identities) == {expected} + + +def test_browser_failure_includes_last_command_detail(tmp_path: Path): + result, output, marker, _ = _run_helper( + tmp_path, + auth_ok=True, + wait_error=True, + ) + + assert result.returncode != 0 + assert "target document did not finish its initial paint" in result.stderr + assert "agent-browser: renderer disconnected" in result.stderr + assert not output.exists() + assert not marker.exists() def test_cold_capture_retries_viewport_until_browser_socket_is_ready(tmp_path: Path): @@ -474,10 +546,14 @@ def test_final_target_reapplies_the_requested_viewport_at_capture_boundary(tmp_p i for i, command in enumerate(commands) if command == "set viewport 412 915" ) - final_screenshot_index = next( + final_screenshot_index = max( i for i, command in enumerate(commands) - if command == f"screenshot {output}" + if command.startswith("screenshot ") ) + final_capture = Path(commands[final_screenshot_index].split(maxsplit=1)[1]) + assert final_capture.parent == output.parent + assert final_capture.name.startswith(".mobius-screenshot.") + assert final_capture.suffix == ".png" target_viewports = [ i for i, command in enumerate(commands) if i > target_index and command == "set viewport 412 915" @@ -512,7 +588,10 @@ def test_capture_primes_the_compositor_before_keeping_evidence(tmp_path: Path): ] assert len(screenshots) == 2 assert "/mobius-screenshot-warmup." in screenshots[0] - assert screenshots[1] == f"screenshot {output}" + final_capture = Path(screenshots[1].split(maxsplit=1)[1]) + assert final_capture.parent == output.parent + assert final_capture.name.startswith(".mobius-screenshot.") + assert final_capture.suffix == ".png" warmup_index = commands.index(screenshots[0]) post_warmup_frame = next( i for i, command in enumerate(commands[warmup_index + 1:], warmup_index + 1) @@ -521,6 +600,48 @@ def test_capture_primes_the_compositor_before_keeping_evidence(tmp_path: Path): assert warmup_index < post_warmup_frame < commands.index(screenshots[1]) +def test_failed_final_capture_preserves_last_known_good_output(tmp_path: Path): + existing = b"last-known-good" + result, output, _, _ = _run_helper( + tmp_path, + auth_ok=True, + existing_output=existing, + screenshot_fail_after_warmup=True, + ) + + assert result.returncode != 0 + assert "remained too busy to capture" in result.stderr + assert output.read_bytes() == existing + assert not list(tmp_path.glob(".mobius-screenshot.*.png")) + assert not list(tmp_path.glob("mobius-screenshot-warmup.*.png")) + assert not list(tmp_path.glob("mobius-agent-browser-error.*")) + + +def test_shared_profile_transaction_waits_before_browser_commands(tmp_path: Path): + with pytest.raises(subprocess.TimeoutExpired): + _run_helper( + tmp_path, + auth_ok=True, + profile_locked=True, + subprocess_timeout=0.5, + ) + + assert not (tmp_path / "browser.log").exists() + + +def test_malformed_app_route_is_rejected_before_capture(tmp_path: Path): + result, output, marker, _ = _run_helper( + tmp_path, + auth_ok=True, + route="/app/42oops", + ) + + assert result.returncode != 0 + assert "require a numeric app id" in result.stderr + assert not output.exists() + assert not marker.exists() + + def test_shell_capture_retries_a_solid_background_frame(tmp_path: Path): result, output, marker, browser_log = _run_helper( tmp_path,