feat: sync account priorities after rate changes
This commit is contained in:
@@ -0,0 +1,123 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user