feat: interactive browser sessions via xpra#178
Conversation
Split the `_capture` method into three helper functions to clarify responsibilities and reduce inline complexity: * `_apply_capture_settings()`: applies all per-capture browser settings (headers, cookies, viewport, credentials, etc.) to a `Capture` instance. This is required by Mypy due to TypedDict assignment constraints. * `_initialize_capture_context()`: wraps `capture.initialize_context()` with a proportional timeout and converts timeouts to `RetryCapture`. * `_run_standard_capture()`: contains the full `Capture` context-manager block previously inline in `_capture`, including browser launch, page capture, error classification, and retry eligibility checks. Returns `(CaptureResponse, should_retry)` so `_capture` retains control over retry and result handling. `_capture` now acts as a thin dispatcher: resolving URL, proxy, and cookies before delegating to `_run_standard_capture`. Observable behavior is unchanged. The `RetryCapture` handler is also improved to populate the result from the exception message when the result is empty, preventing a silent empty error field on retry exhaustion.
…ings Introduce the data model and initial support for interactive browser sessions. helpers.py: * Import SetCookieParam and Cookie directly from playwrightcapture.capture, following the removal of their re-exports from __init__. * Add SessionStatus enum (UNKNOWN / STARTING / READY / ERROR / STOPPED / EXPIRED / CAPTURE_REQUESTED) to mirror the style of CaptureStatus. * Define SessionMetadata TypedDict: backend-agnostic public state stored under lacus:session:<uuid> in Redis, including status, backend_type, view_url, created_at, expires_at, and capture_requested_at. * Define XpraSessionMetadata TypedDict for Xpra transport details stored under lacus:session:<uuid>:xpra (display, socket_path). * Provide StoredSessionMetadata union type to give callers a combined view of both metadata types. * Extend CaptureSettings with interactive: bool = False and interactive_ttl: int = 900. lacuscore.py: * Import SessionStatus for caller-facing exports; it is not used elsewhere in this commit. * Add interactive_allowed: bool = False to LacusCore.__init__ and store as self.interactive_allowed. * Restore enqueue() overloads and docstring with interactive / interactive_ttl parameters. * Add guard to raise CaptureSettingsError if interactive is requested when interactive_allowed is False. __init__.py: * Export SessionStatus.
…ackend
Introduce three modules implementing the interactive session layer.
session.py:
* `Session` dataclass: backend-agnostic representation of a live session (`created_at`, `expires_at`, `view_url`).
* `SessionManager` Protocol: structural contract for session backends, including `start_session`, `stop_session`, `serialize_backend_metadata`, `restore_session`, and `get_capture_kwargs`.
session_store.py:
* `StoredSessionRecord` dataclass: pairs `SessionMetadata` with opaque `backend_metadata`; `.merged_metadata()` produces a `StoredSessionMetadata` view for callers needing the full picture.
* `SessionMetadataStore`: Redis-backed lifecycle store.
* Two key namespaces: `lacus:session:<uuid>` (public state) and `lacus:session:<uuid>:<backend_type>` (transport details).
* `write()`: pipelines core and backend hashes with optional TTL.
* `mark_terminal()`: sets status, clears `capture_requested_at`, and short-expires both keys atomically.
* `read()`: decodes both hashes; falls back to extracting legacy Xpra fields from the core key for backward compatibility.
* `request_finish()`: uses WATCH/MULTI/EXEC to stamp `capture_requested_at` exactly once; idempotent if already set or terminal.
* `scan_expired()`: scans `lacus:session:*` and returns sessions past `expires_at` that are not already terminal.
xpra_session.py:
* `XpraSession` dataclass: extends `Session` with `display` and `socket_path`.
* `XpraSessionManager`: implements the `SessionManager` Protocol for Xpra.
* `start_session()`: launches Xpra with HTML5 on and a Unix socket transport; reads the allocated display via `--displayfd`; waits up to 30 s for the display and 10 s for the socket.
* `stop_session()`: stops the session targeting the socket first, then the display; waits for socket removal; treats already-gone backends as successful.
* `serialize_backend_metadata()`, `restore_session()`, and `get_capture_kwargs()`: persist and restore Xpra transport details through Redis.
* `_build_clean_env()`: allowlists a minimal set of environment variables for a predictable subprocess environment.
* `view_url` is constructed from `LACUS_XPRA_PUBLIC_BASE_URL` (supports `{uuid}` placeholder); left `None` if the variable is unset.
Integrate the interactive session backend with the capture pipeline. LacusCore.**init**: * Instantiate `SessionMetadataStore` and assign to `self.session_store`. Public session management API: * `get_session_metadata(uuid)`: retrieves `SessionMetadata` from Redis. * `get_session_backend_metadata(uuid)`: returns the opaque backend dictionary. * `request_session_capture(uuid)`: optimistically stamps `capture_requested_at`; operation is idempotent. * `stop_session(uuid, status)`: restores the session object, calls `manager.stop_session()`, and marks the session terminal in Redis. * `cleanup_expired_sessions()`: scans `lacus:session:*` and expires sessions past their TTL. Internal helpers: * `_stop_live_session_backend(manager, session)`: executor wrapper for the blocking `stop_session` call. * `_get_session_manager(backend_type)`: factory returning the appropriate session manager, e.g., `XpraSessionManager` for the `'xpra'` backend. * `_open_interactive_page(capture, to_capture, url, logger)`: opens a new page in the running context, navigates to the URL, and brings it to the front; raises `RetryCapture` on failure. * `_run_interactive_capture(...)`: manages the full interactive session lifecycle: starts the Xpra session in an executor, writes `STARTING` metadata to Redis, opens the browser under the session display, marks `READY`, polls for `capture_requested_at` or TTL expiry; on finish, calls `capture.capture_current_page()`, records runtime, and marks terminal `STOPPED`. On exceptions, marks `ERROR` before re-raising. _capture: add interactive dispatch branch before the standard path. Uses a `getattr` guard to maintain compatibility with `CaptureSettings` instances that pre-date the interactive field.
- __init__.py: replace SessionStatus re-export with XpraSessionManager; update __all__ to match. - helpers.py: remove stale "Deprecated" comment on CAPTURE_REQUESTED (capture_requested_at is tracked in SessionMetadata, not via this value). - lacuscore.py: move interactive/interactive_ttl docstring params to before :param headless: to match argument order.
session_store.py: - Add type parameter to Redis -> Redis[bytes] - Replace incorrect cast() on hset mapping args with type: ignore - Annotate pipeline.hgetall() return in WATCH context lacuscore.py: - Import Page and PageCaptureState under TYPE_CHECKING - Narrow _open_interactive_page return from tuple[Any, Any] to tuple[Page, PageCaptureState] - Widen hash_to_set type to dict[str, bytes | int | float | str] and remove stale type: ignore comments - Add cast(SessionMetadata, ...) for dict(record.metadata) returns - Remove redundant casts in stop_session - Add explicit return / assert-unreachable to satisfy missing-return - Add type: ignore[attr-defined] for cross-repo methods pending release - Initialize result in _run_interactive_capture
- Remove spaces around '=' in default args (xpra_session.py, lacuscore.py) to match codebase convention. - Replace getattr(to_capture, 'interactive', False) with direct attribute access; CaptureSettings always defines the field. - Add missing newline at end of session_store.py.
Add one-line docstrings to LacusCore.get_session_metadata and the five public methods on SessionMetadataStore (write, mark_terminal, read, request_finish, scan_expired).
- Mention interactive_ttl parameter (default 900s) - Name request_session_capture(uuid) as the method that triggers the final capture
- Set socket directory mode to 0o700 to prevent world-readable sockets - Add field_validator for interactive_ttl: enforce 1-600 second range - Lower interactive_ttl default from 900 to 600 seconds
Rafiot
left a comment
There was a problem hiding this comment.
Just a few notes, I haven't tested the code yet, more early next week, sorry it took me more time than I expected.
lacuscore/session_store.py
Outdated
| def backend_key(uuid: str, backend_type: str) -> str: | ||
| return f'lacus:session:{uuid}:{backend_type}' | ||
|
|
||
| def _decode_hash(self, raw: dict[bytes, Any]) -> dict[str, Any]: |
There was a problem hiding this comment.
we can probably simplify this one because the key and the values will always be bytes.
There was a problem hiding this comment.
Not sure how I ended up with that... simplified in 629818e - removed isinstance guards.
lacuscore/session_store.py
Outdated
| """Redis-backed storage for interactive session lifecycle and backend state.""" | ||
|
|
||
| _int_fields = {'status', 'created_at', 'expires_at', 'capture_requested_at'} | ||
| _legacy_xpra_fields = {'display', 'socket_path'} |
There was a problem hiding this comment.
Do we need the whole legacy stuff? As far as I can tell, XpraSessionMetadata is the one to use. And the sessions expire, so there is no reason to have the legacy stuff anywhere passed the expiration time.
There was a problem hiding this comment.
100% - the legacy stuff wasn't supposed to be there, leftovers from an earlier refactor (pre PR).
Removed _legacy_xpra_fields, _extract_legacy_backend_metadata(), StoredSessionMetadata, and merged_metadata() in 629818e.
lacuscore/session_store.py
Outdated
| else: | ||
| pipeline.delete(backend_key) | ||
|
|
||
| if expire_seconds is not None: |
There was a problem hiding this comment.
can we always make sure the sessions expire? Even with a very long timeout? From experience, there is always something somewhere that fails and we end up with dangling values in the database, and/or a never ending session.
There was a problem hiding this comment.
Good call - expire_seconds is now a required int parameter on write() (no default, no None). Both callers already passed it, so no downstream changes needed. Done in 629818e.
lacuscore/session_store.py
Outdated
| decoded[key] = value | ||
| return decoded | ||
|
|
||
| def _extract_legacy_backend_metadata(self, core_metadata: dict[str, Any]) -> dict[str, Any]: |
There was a problem hiding this comment.
See legacy, probably not needed?
There was a problem hiding this comment.
Removed in the same commit as above (629818e) - when no backend key exists, read() now returns an empty dict.
| logger.critical('Unable to connect to redis and to push the result of the capture.') | ||
|
|
||
| # Expire stats in 10 days | ||
| expire_time = timedelta(days=10) |
There was a problem hiding this comment.
I was surprised, and the python module accepts a timedelta there: https://valkey-py.readthedocs.io/en/latest/commands.html#valkey.commands.core.CoreCommands.expire
not a big deal, but converting it to seconds in unnecessary.
There was a problem hiding this comment.
TIL — switched to timedelta(days=10) directly in 5cc173d.
| hash_to_set[key] = bytes(value) if isinstance(value, bytearray | memoryview) else value | ||
| else: | ||
| logger.warning(f'Unexpected capture response type for {key}, serializing defensively.') | ||
| hash_to_set[key] = pickle.dumps(value) |
There was a problem hiding this comment.
I'm not 100% sure we want to pickle dump that one in redis: it's (maybe?) safer than was done before before, but it shouldn't be there. We might even want to raise an exception.
I'll dig a bit further.
There was a problem hiding this comment.
Since the CaptureResponse TypedDict has a fixed set of keys, and every one is (supposed to be) already handled, the else branch should be unreachable... except maybe if a new field were added without updating the function. Pickling would hide the issue, but I think that's more of a bug-scenario; something to be surfaced.
Happy to change this to raise? or to log-and-skip?
| stats_pipeline=stats_pipeline, | ||
| today=today, | ||
| ) | ||
| return |
There was a problem hiding this comment.
Not a fan of the return there. Not sure, but we may want to have a should_retry too.
There was a problem hiding this comment.
The bare return is a bit weird. The reason it's there: _run_interactive_capture handles its own result storage, runtime logging, and cleanup internally (via mark_terminal + _stop_live_session_backend), so falling through to the finally block would restore the result redundantly and run cleanup that doesn't apply (retry logic, document unlinking).
I could refactor this to make the flow clearer - e.g. an early if to_capture.interactive: guard before the try block so there's no surprising mid-block exit?
On should_retry - interactive sessions have a slightly different failure model: the user is present and failures are surfaced in the session metadata. Not sure, but I think silently retrying might restart a session the user thinks is still live. I could see value in retrying the final snapshot bit specifically, maybe?
- Drop `PageCaptureState` import (`TypedDict` removed upstream) - `_open_interactive_page()` returns `Page` (`setup_page_capture` returns `None`) - Use `capture_page(current_page_only=True)` instead of `capture_current_page()`
…folding - Simplify `_decode_hash` — `Redis[bytes]` always returns bytes, so the `isinstance` guards are redundant. - Remove `_legacy_xpra_fields`, `_extract_legacy_backend_metadata()`, `StoredSessionMetadata`, and `merged_metadata()` — development leftovers from an earlier refactor pass. - Make `expire_seconds` mandatory on `write()` to ensure sessions always get a TTL.
valkey-py accepts `timedelta` natively — no need to convert to seconds.
Add interactive capture support to LacusCore. When initialized with
interactive_allowed=Trueandheaded_allowed=True, callers can enqueue interactive captures that start a live browser session inside an xpra server. The user interacts with the page (log in, solve CAPTCHAs, navigate) and then triggers a final capture when ready.Changes
New modules
session.py- Backend-agnosticSessiondataclass andSessionManagerprotocol. Defines the contract any future session backend must satisfy.session_store.py-SessionMetadataStorefor persisting interactive session state in Redis with atomic updates (WATCH/MULTI forrequest_finish), TTL-based expiry, andscan_expiredfor cleanup.xpra_session.py-XpraSessionManager, the concrete backend. Starts per-session xpra servers on private unix sockets with a sanitized environment, manages display allocation via--displayfd, and handles graceful shutdown.Modified modules
lacuscore.py- New_run_interactive_capture()async method orchestrating the session lifecycle: start xpra → write metadata → launch browser on the xpra display → poll for finish/expiry → trigger PlaywrightCapture'scapture_current_page()→ stop xpra → return results. Refactored_captureinto focused helpers to share code between standard and interactive flows.helpers.py- NewSessionStatusenum,SessionMetadata/XpraSessionMetadata/StoredSessionMetadataTypedDicts for interactive session state.__init__.py- ExportSessionStatusandXpraSessionManager.Hardening
mode=0o700to prevent world-readable sockets.interactive_ttlvalidated upstream in lookyloo-models via Pydanticfield_validator(1–600 seconds).Documentation
SessionMetadataStoremethods.get_session_metadata/get_session_backend_metadata), andrequest_session_capture().Public API additions
enqueue(..., interactive=True, interactive_ttl=600)str(UUID)get_session_metadata(uuid)SessionMetadata | Noneget_session_backend_metadata(uuid)dict | Nonerequest_session_capture(uuid)SessionMetadata | NoneDependencies
Requires PlaywrightCapture with the
displayparameter andcapture_current_page()method (companion PR: Lookyloo/PlaywrightCapture#187).Requires lookyloo-models with
interactive/interactive_ttlfields onCaptureSettings(companion PR: https://github.com/Lookyloo/lookyloo-models/pulls/1)