fix(browser): preserve sessions and native browser identity

[yunfanye]

What this PR does

Closing Chrome discards session-only site logins even when its profile survives. The CDP guard exits after five idle seconds, configures some targets repeatedly, and can report readiness before startup completes. Browser-level attachment also misses nested cross-origin iframes.

Keeping the guard running exposes another compatibility problem: unconditional shims replace native Chromium properties and fabricate user-agent metadata. Controlled Delta searches returned HTTP 444 with these overrides and HTTP 200 with native values.

Restore browser sessions, repair guard lifecycle and recursive frame attachment, and preserve native browser identity by default. JavaScript shims apply only when the browser reports WebDriver automation. User-agent metadata changes only when a user-agent override is explicitly configured.

Design & Invariants

• The guard remains enabled by default. Its locale, timezone, viewport, and frame setup remain active.
• Native Chromium JavaScript properties and built-ins remain intact when it does not report WebDriver automation. The payload also removes its temporary language configuration global.
• Explicit user-agent overrides and WebDriver shims remain supported.
• Idle event timeouts do not terminate the guard. Command-response timeouts still report failures, and a disconnected browser ends the guard.
• Each page and iframe session watches its children before resuming. The Page agent is enabled for both kinds of document.
• Startup succeeds only after the guard writes its readiness signal.

Test plan

• pnpm typecheck
• pnpm test:unit, including native browser identity, explicit overrides, guard lifecycle, nested frames, and readiness regressions
• bash -n scripts/docker/rome-start-chrome-cdp.sh scripts/docker/rome-apply-cdp-stealth.sh
• Verify idle survival and attachment of a new tab in live Chromium.
• Verify existing and nested OOPIF attachment in isolated Chromium with --enable-automation. Confirm injection before the first frame script, including a nested frame created after idle.
• Compare fresh Delta search submissions using native values, full overrides, and partial overrides. Verify first-submission results and offer API responses with the corrected guard running.
• Confirm that the local Rome container reaches Rome started and remains healthy.
• Recover the existing Delta profile: it still receives HTTP 444 despite the browser correction. A site-data reset requires approval because it removes saved Delta sign-in state.
• pnpm dev:all: blocked by the existing host port 80 conflict between Rome and Traefik. The original Traefik configuration was restored.

Host checks used Node 24 because Nix is unavailable on the test host. The local browser contains the correction. The PR remains a draft while recovery of the existing Delta profile is unverified.

Not in this PR

• Automatic deletion of site data, because it would remove user login state.

[Jessie-QingYu]

🔁 This review has been superseded. See the latest review.

[zoolsher]

🔁 This review has been superseded. See the latest review.

[Jessie-QingYu]

🔁 This review has been superseded. See the latest review.

[zoolsher]

🔁 This review has been superseded. See the latest review.

[Jessie-QingYu]

Code Review: 🛑 REQUEST_CHANGES

This PR hardens the CDP stealth guard and Chrome supervisor: it adds --restore-last-session to preserve session cookies, tolerates idle CDP read timeouts in the guard's main loop while still failing on command timeouts and disconnects, replaces explicit page enumeration with recursive Target.setAutoAttach (covering nested OOPIF iframes), waits on a readiness signal written into a temp directory, and gates the JS stealth payload on navigator.webdriver. The guard lifecycle, ordering (configure-before-resume), and readiness handling are well thought out and covered by a solid new test fixture and six regression tests. The supervisor correctly kills the previous guard on restart via cleanup_browser_instance, and the UA-override simplification is consistent.

The main concern is the stealth-inject.js early-return gate: per MDN, navigator.webdriver is only true under --enable-automation, --headless, or --remote-debugging-port=0. Rome launches headful Chrome on --remote-debugging-port=9223 with none of those flags (verified — no automation/headless flags anywhere, and no other consumer sets webdriver), so webdriver is false and the entire JS stealth layer (plugins, WebGL renderer, screen dims, hardwareConcurrency, deviceMemory, window.chrome, languages) becomes a no-op in the deployed configuration. That is a meaningful, undocumented behavior change relative to the previous unconditional payload and appears to conflict with the "stealth remains enabled by default" invariant. A secondary robustness note: moving auto_attach(session_id) to the first line of configure_target's try block makes it a single point of failure for all page-level stealth.

Verdict: REQUEST_CHANGES — The new navigator.webdriver !== true gate makes the entire JS stealth payload a no-op in Rome's actual (non-automation, port≠0) Chrome configuration, which contradicts the PR's stated "stealth remains enabled by default" invariant and needs confirmation.

3 finding(s) posted as inline comments below.

Severity	Category	File	Title
P1	design	scripts/docker/stealth-inject.js	webdriver gate disables the entire JS stealth payload in production
P2	error-handling	scripts/docker/rome-apply-cdp-stealth.sh	auto_attach as first step makes it a single point of failure for page stealth
P3	design	scripts/docker/rome-start-chrome-cdp.sh	--restore-last-session reopens all prior tabs on unclean restart

---

Automated review by RomeOS Code Review · commit 7a74829

[Jessie-QingYu]

[P1] design — webdriver gate disables the entire JS stealth payload in production

This early return short-circuits the whole payload whenever navigator.webdriver !== true. Per MDN, Chrome only reports webdriver === true with --enable-automation, --headless, or --remote-debugging-port=0. Rome launches headful Chrome with --remote-debugging-port=$CHROME_INTERNAL_PORT (9223) and none of those flags, so in the real deployment navigator.webdriver is false and every JS shim below (plugins, WebGL renderer/vendor, screen dimensions, hardwareConcurrency, deviceMemory, window.chrome, navigator.languages/connection) is skipped. Only the CDP Emulation-domain overrides (geo/tz/locale/UA) remain active.

That is a significant, undocumented behavior change from the previous unconditional payload and appears to contradict the PR's "Stealth remains enabled by default" invariant — the JS stealth layer is effectively off. Please confirm intent: if the goal is truly "only patch browsers that already leak automation," state that explicitly and consider whether shipping the (now largely dead) payload is still worthwhile; if the JS shims are meant to run in Rome's normal CDP browser, this gate needs to be removed or driven by an explicit config flag rather than navigator.webdriver.

[Jessie-QingYu]

[P2] error-handling — auto_attach as first step makes it a single point of failure for page stealth

auto_attach(session_id) is now the first send inside configure_target's try. If this call raises (e.g. setAutoAttach unsupported/rejected on a given session, or a transient command timeout), control jumps to the except, so the page never receives geolocation/timezone/locale/UA overrides or the stealth script — a strictly worse outcome than merely missing nested-iframe coverage. Consider isolating the child-watch setup so its failure degrades only OOPIF coverage:

if target_type in {"page", "iframe"}:
    try:
        auto_attach(session_id)
    except Exception as exc:
        log(f"auto-attach failed on {target_type} {target_id}: {exc}")

This keeps the core per-page stealth (which is the primary goal) resilient to auto-attach hiccups.

[Jessie-QingYu]

[P3] design — --restore-last-session reopens all prior tabs on unclean restart

--restore-last-session is the right lever for persisting session cookies, but note it also restores the full previous tab set. Because the supervisor auto-restarts Chrome on every exit (including crashes), a crash loop will repeatedly reopen whatever heavy pages were open, and the guard only creates an about:blank tab when no page is restored. This is likely acceptable, but worth confirming it doesn't resurrect a crashing tab in a restart loop; if that's a concern, pairing with periodic session pruning or --restore-last-session only on clean starts would be safer.

[zoolsher]

Code Review: 🛑 REQUEST_CHANGES

This PR improves session persistence, removes duplicate target attachment, recursively configures nested OOPIFs, preserves native browser identity by default, and keeps the guard alive during idle WebSocket timeouts. The overall recursive auto-attachment direction is sound, but two startup paths still violate the stated stealth/readiness guarantees.

I read all seven changed files in full and traced the launcher through both production and development entrypoints and its CDP consumers. Shell syntax and the Python fixture compile check passed; the Rstest suite could not be run because dependencies are absent from the supplied clone.

Verdict: REQUEST_CHANGES — The readiness gate still exposes an unguarded CDP window, and restored documents can be declared configured without receiving the stealth payload.

3 finding(s) posted as inline comments below.

Severity	Category	File	Title
P1	correctness	scripts/docker/rome-apply-cdp-stealth.sh	Run the payload in restored execution contexts
P1	race-condition	scripts/docker/rome-start-chrome-cdp.sh	Expose the external CDP endpoint only after guard readiness
P2	resource-management	scripts/docker/rome-start-chrome-cdp.sh	Avoid accumulating the startup URL across restored sessions

---

Automated review by RomeOS Code Review · commit 7a74829

[zoolsher]

[P1] correctness — Run the payload in restored execution contexts

Existing targets attached during session restoration are not paused, and Page.addScriptToEvaluateOnNewDocument defaults runImmediately to false, so their live contexts never receive the payload even though the guard logs them as configured and signals readiness. This leaves supported WebDriver-mode restored pages exposing navigator.webdriver === true until their next navigation. Pass runImmediately: True (or explicitly evaluate/reload the current context) and add an existing-context assertion to the fixture test; see the CDP behavior.

[zoolsher]

[P1] race-condition — Expose the external CDP endpoint only after guard readiness

The new ready file gates start_chrome_instance, but ensure_cdp_proxy still exposes port 9222 before start_stealth_guard runs, and production launches this supervisor asynchronously. A consumer can therefore create or navigate a page before automatic attachment is installed, bypassing the startup guarantee. Start and await the guard on the internal port first, then start and verify the external proxy.

[zoolsher]

[P2] resource-management — Avoid accumulating the startup URL across restored sessions

Chrome is always launched with both --restore-last-session and the positional CHROME_URL (default about:blank). Chromium supplies command-line tabs to session restoration, so each supervisor restart can restore the previous blank tab and append another, causing tabs and renderer resources to accumulate. Pass the startup URL only for a new/empty session, or otherwise suppress it when restoring.