SmartUp/remote-browser-autofill-fix-plan.md

# Plan: Remote Browser Autofill Fix

**Generated**: 2026-05-15
**Estimated Complexity**: Medium

## Overview
Remote browser custom pages already store autofill credentials and pass them into `BrowserSessionService.create`, but the service only attempts autofill once immediately after `domcontentloaded`. Modern login pages often render inputs after SPA hydration or behind short async delays, so the first attempt can miss the fields and silently skip.

Fix the root cause by making remote-browser autofill wait/retry for the login fields for a bounded time, then fill the username/password once both locators are visible. Keep the implementation limited to remote browser mode and avoid exposing stored passwords in API responses or logs.

## Prerequisites
- Existing backend virtualenv/dependencies available for tests.
- Playwright APIs remain the same as currently used by `backend/app/services/browser_session_service.py`.
- No changes to stored credential schema are required.

## Sprint 1: Backend Autofill Reliability
**Goal**: Make autofill robust when login fields appear shortly after initial page load.
**Demo/Validation**:
- Unit tests demonstrate delayed locator availability is retried.
- Existing autofill behavior remains unchanged when fields are immediately available.
- No password value is logged or returned from APIs.

### Task 1.1: Add focused autofill behavior tests
- **Location**: `backend/test_browser_session_service.py`
- **Description**: Add async/unit-style tests around `BrowserSessionService._autofill_login` using fake page/locator objects. Cover delayed selector visibility and disabled/missing credential config.
- **Dependencies**: None
- **Acceptance Criteria**:
  - Delayed-field test fails against current one-shot implementation.
  - Test asserts both username and password locators are filled after retry.
  - Disabled/missing config returns without filling.
  - Tests do not require launching real Chromium.
- **Validation**:
  - `cd backend && pytest test_browser_session_service.py`

### Task 1.2: Default-enable autofill when credentials are saved
- **Location**: `backend/app/routers/custom_pages.py`
- **Description**: When creating or updating a custom page with a non-empty login password and username, default `login_autofill_enabled` to true unless the request explicitly sets it to false. This prevents the common state where credentials are saved but the fill path is disabled.
- **Dependencies**: Task 1.1
- **Acceptance Criteria**:
  - New pages with username/password saved execute autofill by default.
  - Updating an existing page with a new password and username enables autofill unless explicitly disabled.
  - Existing pages without credentials are not enabled automatically.
  - Manual switch-off remains respected when explicitly sent.
- **Validation**:
  - Add or extend API/router tests if practical, otherwise verify via focused unit test or direct route behavior.

### Task 1.3: Implement bounded retry/wait for autofill fields
- **Location**: `backend/app/services/browser_session_service.py`
- **Description**: Replace the single locator lookup in `_autofill_login` with a bounded retry helper that repeatedly checks the configured/default selectors until both username and password fields are visible or a deadline expires.
- **Dependencies**: Task 1.1
- **Acceptance Criteria**:
  - Autofill retries for a short bounded window, e.g. up to 8 seconds with small sleeps.
  - It fills only after both username and password locators are found.
  - Existing custom selectors still have highest priority.
  - Missing username/password config still returns immediately.
  - Failed/missing fields still log only generic skip information without secrets.
- **Validation**:
  - `cd backend && pytest test_browser_session_service.py`

### Task 1.4: Preserve reload behavior scope
- **Location**: `backend/app/services/browser_session_service.py`
- **Description**: Keep reload/back/forward event behavior unchanged unless tests prove it is part of the root cause. The initial fix should target the known missed-on-load path only.
- **Dependencies**: Task 1.3
- **Acceptance Criteria**:
  - No unrelated navigation or UI-event behavior changes.
  - No frontend API contract changes.
- **Validation**:
  - Code review and existing tests.

## Sprint 2: Verification and Manual Check
**Goal**: Confirm the fix is safe and usable in the app.
**Demo/Validation**:
- Backend tests pass.
- Frontend type/build check passes if dependencies are present.
- Manual remote-browser check is attempted when the dev stack/browser runtime can be started.

### Task 2.1: Run backend regression tests
- **Location**: `backend/`
- **Description**: Run the new focused test and the existing backend tests relevant to service behavior.
- **Dependencies**: Sprint 1 complete
- **Acceptance Criteria**:
  - New autofill test passes.
  - Existing backend tests pass or any unrelated failures are documented.
- **Validation**:
  - `cd backend && pytest test_browser_session_service.py test_upstream.py test_website_client.py test_group_binding_create_sync.py`

### Task 2.2: Run frontend validation if feasible
- **Location**: `frontend/`
- **Description**: Run available type/build checks to ensure no accidental frontend impact.
- **Dependencies**: Sprint 1 complete
- **Acceptance Criteria**:
  - Frontend build/typecheck passes, or dependency/environment blockers are documented.
- **Validation**:
  - Inspect `frontend/package.json` scripts and run the appropriate check.

### Task 2.3: Manual remote browser verification
- **Location**: App UI, custom page with `access_mode = remote_browser`
- **Description**: Start the app stack if feasible, open a custom page with saved credentials, and verify username/password fields populate after the login form appears.
- **Dependencies**: Sprint 1 complete
- **Acceptance Criteria**:
  - A remote browser page with delayed login fields gets filled.
  - If a submit selector is configured, submit still works after fill.
  - If runtime dependencies are unavailable, document exactly what could not be verified.
- **Validation**:
  - Manual browser check or documented blocker.

## Testing Strategy
- Prefer a focused fake-object unit test for `_autofill_login` so the timing regression is deterministic and does not require Chromium.
- Run relevant existing backend pytest files to catch regressions.
- Attempt frontend build/typecheck because the user-facing flow crosses frontend/backend even though the code change is expected to be backend-only.
- Attempt a manual UI check only after automated validation, and clearly report if Playwright/browser runtime or local services prevent it.

## Potential Risks & Gotchas
- **Autofill switch disabled**: Current DB inspection showed page `id=1` has username/password configured but `login_autofill_enabled=0`; users must enable the switch for autofill to execute.
- **Delayed fields inside iframes**: The retry only covers fields in the main page. If the target site renders login inside an iframe, a separate frame-aware enhancement would be needed.
- **Login appears only after user action**: Retry cannot fill a modal that is opened only after clicking a login button unless selectors point to already visible inputs after navigation.
- **Sites with anti-automation behavior**: Playwright fill may still be blocked by site-specific scripts; this fix handles timing, not hostile automation detection.
- **Persistent profile reuse**: If a reused session is already on a non-login page, autofill should not force navigation or overwrite unrelated inputs.
- **Selector ambiguity**: Default selectors may match the wrong visible text input on unusual pages. Custom selectors remain the mitigation.

## Rollback Plan
- Revert changes to `backend/app/services/browser_session_service.py` and remove `backend/test_browser_session_service.py`.
- Restart the backend to clear any in-memory browser sessions if needed.