Skip to content

feat: interactive browser sessions via xpra#178

Merged
Rafiot merged 14 commits intoail-project:mainfrom
UCD-CCI:feature/interactive-sessions
Apr 10, 2026
Merged

feat: interactive browser sessions via xpra#178
Rafiot merged 14 commits intoail-project:mainfrom
UCD-CCI:feature/interactive-sessions

Conversation

@DocArmoryTech
Copy link
Copy Markdown
Contributor

Add interactive capture support to LacusCore. When initialized with interactive_allowed=True and headed_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-agnostic Session dataclass and SessionManager protocol. Defines the contract any future session backend must satisfy.
  • session_store.py - SessionMetadataStore for persisting interactive session state in Redis with atomic updates (WATCH/MULTI for request_finish), TTL-based expiry, and scan_expired for 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's capture_current_page() → stop xpra → return results. Refactored _capture into focused helpers to share code between standard and interactive flows.
  • helpers.py - New SessionStatus enum, SessionMetadata/XpraSessionMetadata/StoredSessionMetadata TypedDicts for interactive session state.
  • __init__.py - Export SessionStatus and XpraSessionManager.

Hardening

  • Socket directory created with mode=0o700 to prevent world-readable sockets.
  • interactive_ttl validated upstream in lookyloo-models via Pydantic field_validator (1–600 seconds).

Documentation

  • Docstrings on all public SessionMetadataStore methods.
  • README section covering interactive session usage, the two-layer metadata split (get_session_metadata / get_session_backend_metadata), and request_session_capture().

Public API additions

Method Returns Description
enqueue(..., interactive=True, interactive_ttl=600) str (UUID) Enqueue an interactive capture
get_session_metadata(uuid) SessionMetadata | None Public session state (status, view_url, timestamps)
get_session_backend_metadata(uuid) dict | None Trusted transport data (socket path, display)
request_session_capture(uuid) SessionMetadata | None Trigger the final capture

Dependencies

Requires PlaywrightCapture with the display parameter and capture_current_page() method (companion PR: Lookyloo/PlaywrightCapture#187).
Requires lookyloo-models with interactive/interactive_ttl fields on CaptureSettings (companion PR: https://github.com/Lookyloo/lookyloo-models/pulls/1)

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
Copy link
Copy Markdown
Collaborator

@Rafiot Rafiot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few notes, I haven't tested the code yet, more early next week, sorry it took me more time than I expected.

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]:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably simplify this one because the key and the values will always be bytes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how I ended up with that... simplified in 629818e - removed isinstance guards.

"""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'}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

else:
pipeline.delete(backend_key)

if expire_seconds is not None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

decoded[key] = value
return decoded

def _extract_legacy_backend_metadata(self, core_metadata: dict[str, Any]) -> dict[str, Any]:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See legacy, probably not needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of the return there. Not sure, but we may want to have a should_retry too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@Rafiot Rafiot merged commit 5cc173d into ail-project:main Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants