Skip to content

Fix shared browser session resets and add readiness checks - #8

Merged
Steel-tech merged 1 commit into
mainfrom
fix/browser-readiness-validation
Sep 14, 2026
Merged

Fix shared browser session resets and add readiness checks#8
Steel-tech merged 1 commit into
mainfrom
fix/browser-readiness-validation

Conversation

@Steel-tech

@Steel-tech Steel-tech commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Opening the viewer against an existing agent-browser 0.33.2 session could restart its daemon and discard the browser: renderer reads and launcher navigation injected a different idle timeout. Commands now inherit the caller's configuration unchanged. Streams also register their frame handler before the handshake completes and fall back to screenshots if no image arrives within five seconds.

Adds npm run build, npm run validate, npm run doctor, and mandatory npm run test:integration. Test discovery stays within this checkout rather than traversing nested worktrees on Node 20. Real-browser tests use a local HTTP fixture and cover Chromium launch/cleanup, shared-session streaming, navigation, console delivery, and reconnect. The readiness assessment explains the existing Chromium options and prioritizes follow-up work.

Validation:

  • npm run validate: build, ShellCheck, manifest checks, and 306 passing tests on Node 24; one intentionally skipped Node-without-WebSocket case.
  • Node 20 suite: 282 passing, 25 capability-gated skips, no failures.
  • Mandatory real-browser tests on Node 22 and Node 24: both pass, no skips.
  • Real terminal smoke: launched Chromium, entered a local URL, rendered via chafa, and quit successfully.
  • npm run doctor and git diff --check pass.

Daemon idle timeouts now follow engine/caller defaults instead of an implicit 30-minute override. Doctor checks prerequisites, not endpoint reachability or installed engine health. Remaining limitations, including HTTPS discovery and later stream stalls, are documented in docs/readiness.md.

Summary by CodeRabbit

  • New Features

    • Added a diagnostics command to check required tools, browser availability, and connection options.
    • Added build, validation, and integration-test commands for verifying project readiness.
  • Bug Fixes

    • Live streaming now falls back to screenshot polling when no usable frame arrives within five seconds and can retry afterward.
    • Browser daemon idle-timeout settings are now preserved from the caller’s configuration instead of being overridden.
  • Documentation

    • Updated setup instructions, configuration details, streaming behavior, and browser readiness guidance.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5a482b5c-9e2c-4f63-a21e-fe7c1ffddde2

📥 Commits

Reviewing files that changed from the base of the PR and between 6be66c1 and 8cf8fc2.

📒 Files selected for processing (11)
  • README.md
  • bin/doctor.mjs
  • bin/renderer.mjs
  • docs/readiness.md
  • package.json
  • scripts/build.mjs
  • scripts/open.sh
  • tests/doctor.test.mjs
  • tests/launch.integration.test.mjs
  • tests/launchers.test.mjs
  • tests/renderer.test.mjs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Changes

The PR adds live-stream frame recovery and five-second polling fallback. It preserves caller-configured agent-browser daemon timeouts. It adds browser diagnostics, syntax and repository validation, explicit test commands, integration-test enforcement, and readiness documentation.

Browser readiness

Layer / File(s) Summary
Live-stream recovery
bin/renderer.mjs, tests/renderer.test.mjs, README.md
The renderer handles frames received during the WebSocket handshake and falls back to screenshot polling after five seconds without an image. Tests cover handshake delivery, timeout cleanup, navigation, reconnects, and local fixture serving.
Daemon configuration preservation
bin/renderer.mjs, scripts/open.sh, tests/launchers.test.mjs, tests/renderer.test.mjs, README.md
The renderer and launcher no longer inject a default idle timeout. Tests verify that unset and caller-provided values reach agent-browser unchanged.
Diagnostics and validation workflow
bin/doctor.mjs, scripts/build.mjs, package.json, tests/doctor.test.mjs, tests/launch.integration.test.mjs, docs/readiness.md, README.md
The package adds prerequisite diagnostics, syntax and repository checks, explicit test scripts, integration-test enforcement, and readiness documentation.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: claude

Sequence Diagram(s)

sequenceDiagram
  participant Renderer
  participant ChromeWebSocket
  participant ScreenshotPolling
  Renderer->>ChromeWebSocket: install frame handler and open stream
  ChromeWebSocket-->>Renderer: deliver image frame or remain quiet
  Renderer->>ScreenshotPolling: fall back after five seconds without an image
Loading

Merge Risk: ⚪ Minimal · up to 8cf8f

Validation intentionally permits unavailable optional browser prerequisites, while the separate integration command enforces them. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 8 files. (3 skipped: 3… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the two main changes: preserving shared browser sessions and adding readiness checks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 8 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/browser-readiness-validation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Steel-tech
Steel-tech merged commit 952e160 into main Sep 14, 2026
5 checks passed
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.

1 participant