Skip to content
This repository was archived by the owner on Aug 24, 2026. It is now read-only.
This repository was archived by the owner on Aug 24, 2026. It is now read-only.

[qa-focus-workbench] Latent: hidden-browser engine state not reset across newPage / engine reuse #24

Description

@amazon7737

Cross-repo note: This is about the separate qa-focus-workbench project (a desktop app embedding a devtools-qa-runner BrowserEngine in a hidden Electron BrowserWindow). It has no GitHub repo of its own yet, so it is filed here on the runner's tracker as an umbrella, per request. The referenced code lives in qa-focus-workbench/src/electron-runner-engine.mjs, not in this repo.

Summary

ElectronRunnerEngine accumulates per-page observer state (consoleMessages, plus the network request map/entries) but never resets it when a new page opens. newPage() calls stop(), creates a fresh BrowserWindow, and re-runs attachObservers(), yet the collected arrays/maps carry over.

Why this is NOT currently an active bug

Traced the full data flow before filing:

  • runQa calls engine.newPage(url) exactly once per run (src/core/runner.mjs).
  • qa-focus-workbench builds a fresh ElectronRunnerEngine per run (engineFactory in src/main.mjs), so nothing carries across runs.
  • The runner collects console/network with includePreservedMessages/Requests: true, i.e. it wants cumulative history for the single page.

So today the state only accumulates within one page of one run (correct), and the request map is bounded by that run's request count (the instance is discarded afterward). No cross-run leak, no incorrect data.

When it WOULD bite (the latent risk)

If an engine instance is ever reused for multiple newPage() calls (permitted by the BrowserEngine contract, though no current caller does it):

  • console/network entries from a previous page would leak into the next page's report;
  • the request map would grow across pages with no upper bound.

Proposed fix (defensive, low-risk)

Reset collectors at the start of newPage(), right after await this.stop(), so each page starts clean:

await this.stop();
this.consoleMessages = [];
this.networkLog = new NetworkLog();
// ...then create BrowserWindow, attachObservers, loadUrl

Context

Found while investigating two suspected engine bugs. The other was real and already fixed in qa-focus-workbench: CDP Network.loadingFailed carries no URL, so the handler had been putting requestId into the url field, breaking ignoreUrlIncludes in both assert-no-http-errors and the quality gate. That mapping was extracted into a unit-tested NetworkLog (which the reset above re-instantiates). This issue tracks only the remaining latent state-reset hardening.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions