From 1acefd92fa3997e56fa42e14a2993f07a874316c Mon Sep 17 00:00:00 2001 From: Tom Softreck Date: Fri, 2 Oct 2026 11:08:41 +0200 Subject: [PATCH] fix(gui): reuse active browser session on subsequent GUI_START (ticket-024) --- project/TICKETS.md | 1 + project/ticket-024/README.md | 27 +++++++ project/ticket-024/intent.json | 85 ++++++++++++++++++++++ testql/interpreter/_gui.py | 39 ++++++++++ tests/test_gui_reuse_session.py | 121 ++++++++++++++++++++++++++++++++ 5 files changed, 273 insertions(+) create mode 100644 project/ticket-024/README.md create mode 100644 project/ticket-024/intent.json create mode 100644 tests/test_gui_reuse_session.py diff --git a/project/TICKETS.md b/project/TICKETS.md index 664d950..2be9006 100644 --- a/project/TICKETS.md +++ b/project/TICKETS.md @@ -29,4 +29,5 @@ This file indexes governance tickets without taking ownership of | **ticket-021** | [`README.md`](./ticket-021/README.md) | - | - | - | - | - | | **ticket-022** | [`README.md`](./ticket-022/README.md) | - | - | - | - | - | | **ticket-023** | [`README.md`](./ticket-023/README.md) | - | - | [`ai-codex.md`](./ticket-023/ai-codex.md) | - | - | +| **ticket-024** | [`README.md`](./ticket-024/README.md) | - | - | - | - | - | diff --git a/project/ticket-024/README.md b/project/ticket-024/README.md new file mode 100644 index 0000000..8c983da --- /dev/null +++ b/project/ticket-024/README.md @@ -0,0 +1,27 @@ +# Ticket 024: Reuse active GUI session on subsequent GUI_START + +- **ID**: ticket-024 +- **Owner**: agent:antigravity +- **Status**: IN_PROGRESS +- **Workflow state**: EDIT +- **Created**: 2026-10-02 + +## Goal and scope + +Allow `GUI_START` to safely reuse an existing active browser session (or navigate to new `app_path`) +when called multiple times (such as across `INCLUDE` child scenarios), avoiding Playwright sync loop +concurrency errors. + +## Acceptance criteria + +- [x] AC-01: `GUI_START` reuses open browser session and navigates if page is open. +- [x] AC-02: Unit test `tests/test_gui_reuse_session.py` passes. +- [x] AC-03: Governance checks pass. + +## Authorization + +SESSION_EXECUTION_AUTHORIZATION: User requested sequential testing of all functionalities with testql, fixing the library directly if GUI testing fails, and merging the changes. + +## Evidence boundary + +Operational test receipts and browser execution logs remain local state. diff --git a/project/ticket-024/intent.json b/project/ticket-024/intent.json new file mode 100644 index 0000000..f8658fb --- /dev/null +++ b/project/ticket-024/intent.json @@ -0,0 +1,85 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-024", + "summary": "Reuse active GUI session on subsequent GUI_START", + "workstream": "core", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "regression" + }, + "allowedPaths": [ + "project/ticket-024/**", + "TODO.md", + "project/TICKETS.md", + "testql/interpreter/_gui.py", + "tests/test_gui_reuse_session.py" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [ + "python" + ], + "dependsOn": [ + "ticket-023" + ], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "acceptedBaseSha": "14bf7e417727ee66e04313f3ad19e5f13d5e3f69", + "targetBranch": "main", + "outcome": "Allow GUI_START to safely reuse an existing active browser session or navigate to new URL without raising Playwright sync loop concurrency errors.", + "nonGoals": [ + "No changes to desktop driver", + "No changes to CDP attachment protocol" + ], + "complexity": "XS", + "estimatedMinutes": 10, + "budgets": { + "maxImplementationFiles": 2, + "maxAffectedComponents": 1, + "maxPublicInterfaceChanges": 0, + "maxRuntimeDependencies": 0 + }, + "architecture": { + "status": "accepted", + "decision": "In _start_playwright and _start_selenium, check if an active page/app session exists and is open. If open, reuse and navigate to app_path instead of launching a second sync_playwright instance; if closed, close session cleanly before starting a new one.", + "components": [ + { + "name": "gui-session", + "paths": [ + "testql/interpreter/_gui.py", + "tests/test_gui_reuse_session.py" + ] + } + ], + "responsibilityChanges": false, + "interfaceChanges": [], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Revert GUI session reuse in _start_playwright." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-01", + "commands": [ + "pytest tests/test_gui_reuse_session.py" + ], + "evidence": "Subsequent GUI_START commands reuse the active Playwright session without raising exceptions." + }, + { + "criterion": "AC-02", + "commands": [ + "bash project/governance-check.sh" + ], + "evidence": "Governance check passes." + } + ] + } +} diff --git a/testql/interpreter/_gui.py b/testql/interpreter/_gui.py index a7ea6f7..bd832b4 100644 --- a/testql/interpreter/_gui.py +++ b/testql/interpreter/_gui.py @@ -366,6 +366,31 @@ def _start_playwright(self, app_path: str, extra_args: str) -> None: operation_timeout = self._gui_operation_timeout() navigation_timeout = self._gui_operation_timeout(15000) + if self._gui_page is not None and self._gui_app is not None: + try: + is_closed = False + if hasattr(self._gui_page, "is_closed"): + is_closed = self._gui_page.is_closed() + if not is_closed: + if app_path and app_path not in ("current", ".", "about:blank"): + if not self._same_url_without_hash(getattr(self._gui_page, "url", ""), app_path): + try: + self._gui_page.goto(app_path, timeout=navigation_timeout) + except Exception as nav_error: + if "net::ERR_ABORTED" not in str(nav_error): + raise + self.out.step("🖥️", f"Playwright: Reused session, opened {app_path}") + self.results.append(StepResult( + name=f'GUI_START "{app_path[:40]}"', status=StepStatus.PASSED + )) + return + else: + self._close_gui_session() + except Exception: + self._close_gui_session() + elif self._gui_app is not None: + self._close_gui_session() + if cdp_url: if self._gui_playwright_backend == "node": raise RuntimeError( @@ -504,6 +529,20 @@ def _start_selenium(self, app_path: str, extra_args: str) -> None: ) app_path = f"{base_url.rstrip('/')}/{app_path.lstrip('/')}" if app_path else base_url + if self._gui_page is not None and self._gui_app is not None: + try: + if app_path.startswith(("http://", "https://", "about:", "file://")): + self._gui_app.get(app_path) + self.out.step("🖥️", f"Selenium: Reused session, opened {app_path}") + self.results.append(StepResult( + name=f'GUI_START "{app_path[:40]}"', status=StepStatus.PASSED + )) + return + except Exception: + self._close_gui_session() + elif self._gui_app is not None: + self._close_gui_session() + if app_path.startswith(("http://", "https://", "about:", "file://")): # Web app headless = str(self.vars.get("headless", "true")).lower() == "true" diff --git a/tests/test_gui_reuse_session.py b/tests/test_gui_reuse_session.py new file mode 100644 index 0000000..13333d9 --- /dev/null +++ b/tests/test_gui_reuse_session.py @@ -0,0 +1,121 @@ +"""Unit tests for reusing active GUI sessions across GUI_START invocations.""" + +from __future__ import annotations + +import pytest + +from testql.interpreter import OqlInterpreter + + +def test_start_playwright_reuses_active_page() -> None: + """Test _start_playwright reuses an already active open page and navigates.""" + interp = OqlInterpreter(api_url="http://example.com:8100", dry_run=False, quiet=True) + interp._gui_driver = "playwright" + interp._gui_playwright_backend = "python" + + navigated_urls: list[str] = [] + + class MockPage: + url = "http://example.com:8100/initial" + + def is_closed(self) -> bool: + return False + + def goto(self, url: str, **kwargs: object) -> None: + navigated_urls.append(url) + self.url = url + + class MockBrowser: + closed = False + + def close(self) -> None: + self.closed = True + + class MockPlaywright: + stopped = False + + def stop(self) -> None: + self.stopped = True + + mock_page = MockPage() + mock_browser = MockBrowser() + mock_playwright = MockPlaywright() + + interp._gui_page = mock_page + interp._gui_app = (mock_playwright, mock_browser) + + # Calling _start_playwright should reuse existing session and navigate + interp._start_playwright("/connect-test", "") + + assert navigated_urls == ["http://example.com:8100/connect-test"] + assert interp.results[-1].status.value == "passed" + assert mock_browser.closed is False + assert mock_playwright.stopped is False + + +def test_start_playwright_recovers_if_page_is_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """Test _start_playwright safely closes stale session if is_closed returns True.""" + interp = OqlInterpreter(api_url="http://example.com:8100", dry_run=False, quiet=True) + interp._gui_driver = "playwright" + interp._gui_playwright_backend = "python" + + class ClosedPage: + url = "http://example.com:8100/dead" + + def is_closed(self) -> bool: + return True + + class OldBrowser: + closed = False + + def close(self) -> None: + self.closed = True + + class OldPlaywright: + stopped = False + + def stop(self) -> None: + self.stopped = True + + old_browser = OldBrowser() + old_playwright = OldPlaywright() + interp._gui_page = ClosedPage() + interp._gui_app = (old_playwright, old_browser) + + new_opened_urls: list[str] = [] + + class NewPage: + url = "" + + def set_default_timeout(self, t: int) -> None: + pass + + def set_default_navigation_timeout(self, t: int) -> None: + pass + + def goto(self, url: str, **kwargs: object) -> None: + new_opened_urls.append(url) + + class NewBrowser: + def new_page(self) -> NewPage: + return NewPage() + + class NewChromium: + def launch(self, **kwargs: object) -> NewBrowser: + return NewBrowser() + + class NewPlaywright: + chromium = NewChromium() + + def start(self) -> NewPlaywright: + return self + + import playwright.sync_api + monkeypatch.setattr(playwright.sync_api, "sync_playwright", lambda: NewPlaywright()) + + interp._start_playwright("/connect-test", "") + + assert old_browser.closed is True + assert old_playwright.stopped is True + assert new_opened_urls == ["http://example.com:8100/connect-test"] + assert interp.results[-1].status.value == "passed"