Skip to content

Improve browser tool cleanup, PATH setup, and screenshot recovery#1001

Open
davetist wants to merge 2 commits intoNousResearch:mainfrom
davetist:codex-browser-cleanup-fixes
Open

Improve browser tool cleanup, PATH setup, and screenshot recovery#1001
davetist wants to merge 2 commits intoNousResearch:mainfrom
davetist:codex-browser-cleanup-fixes

Conversation

@davetist
Copy link

@davetist davetist commented Mar 12, 2026

Summary

This PR hardens the browser tool in three areas:

  • cleanup is now consistent across manual close, inactivity cleanup, and emergency shutdown
  • browser command execution is more reliable in mixed/local installs, especially around PATH, Node resolution, and local socket usage
  • screenshot capture and recovery is more robust, including the fallback path when agent-browser emits human-readable output instead of JSON

What Changed

Session lifecycle and cleanup

  • changed browser_close() to delegate to cleanup_browser() instead of doing a partial inline teardown
  • updated _emergency_cleanup_all_sessions() to route through cleanup_all_browsers() and clear in-memory tracking state after shutdown
  • ensured cleanup consistently handles:
    • recording shutdown
    • Browserbase session release
    • _active_sessions removal
    • _session_last_activity removal
    • local daemon PID/socket cleanup
    • local socket-directory removal
  • kept the inactivity cleanup thread behavior, but made the teardown path it triggers match the explicit/manual cleanup path

Local session reliability

  • shortened local session names to reduce Unix socket path pressure
  • continued using /tmp on macOS via _socket_safe_tmpdir() to avoid long AF_UNIX socket paths under /var/folders/...
  • these changes reduce the chance of local agent-browser daemon/socket failures on macOS

Browser command execution

  • updated _run_browser_command() to build a safer subprocess PATH
  • the browser subprocess now prefers Hermes-managed Node from $HERMES_HOME/node/bin before standard system directories
  • also ensures common system directories are present even in minimal service environments
  • logs which node binary is actually being used, which makes environment/debugging issues easier to diagnose

Non-JSON output handling

  • tightened handling when agent-browser returns non-JSON output
  • instead of treating arbitrary non-JSON output as success, the browser tool now treats it as an error by default
  • added a screenshot-specific recovery path so human-readable screenshot output can still be salvaged when a real PNG file was created

Screenshot capture and vision flow

  • fixed _extract_screenshot_path_from_text() so it matches real absolute PNG paths, including quoted paths, such as:
    • /tmp/foo.png
    • /Users/david/.hermes/browser_screenshots/shot.png
  • updated browser_vision() to request a full-page screenshot with --full
  • if agent-browser returns a different screenshot path in its JSON payload, browser_vision() now uses that actual path instead of assuming the original requested path
  • improved error reporting when screenshot capture appears to succeed but the expected file is missing
  • preserves captured screenshots when the vision-model step fails, so the user can still inspect or share the image

Upstream merge / conflict resolution

  • merged the latest upstream/main into this branch to resolve the open PR conflict
  • kept the new upstream call_llm(...) integration for:
    • snapshot summarization
    • browser_vision() analysis
  • reapplied the cleanup, PATH, session, and screenshot-recovery fixes on top of that newer implementation

Tests

  • added regression coverage in tests/tools/test_browser_cleanup.py for:
    • screenshot path extraction
    • cleanup_browser() state cleanup
    • browser_close() delegation
    • emergency cleanup state clearing

Verification

Passed:

  • python -m pytest tests/tools/test_browser_cleanup.py -q
  • python -m pytest tests/tools/test_browser_console.py tests/gateway/test_send_image_file.py -q
  • python -m pytest tests/tools/test_browser_cleanup.py tests/tools/test_browser_console.py tests/gateway/test_send_image_file.py -q

Full Suite Note

I also ran the full suite on this branch and on main with:

  • python -m pytest tests/ -q

The full suite is already red on main, with widespread failures outside this change area, so this PR is not introducing a uniquely broken full-suite state.

Unify browser session teardown so manual close, inactivity cleanup, and emergency shutdown all follow the same cleanup path instead of partially duplicating logic.

This changes browser_close() to delegate to cleanup_browser(), which means recording shutdown, Browserbase release, activity bookkeeping cleanup, and local socket-directory removal now happen consistently. It also updates emergency cleanup to route through cleanup_all_browsers() and explicitly clear in-memory tracking state after teardown so stale active-session, last-activity, and recording entries are not left behind on exit.

The screenshot fallback path has also been fixed. _extract_screenshot_path_from_text() now matches real absolute PNG paths, including quoted output, so browser_vision() can recover screenshots when agent-browser emits human-readable text instead of JSON.

Regression coverage was added in tests/tools/test_browser_cleanup.py for screenshot path extraction, cleanup_browser() state removal, browser_close() delegation, and emergency cleanup state clearing.

Verified with:
- python -m pytest tests/tools/test_browser_cleanup.py -q
- python -m pytest tests/tools/test_browser_console.py tests/gateway/test_send_image_file.py -q
@davetist davetist force-pushed the codex-browser-cleanup-fixes branch from 117b984 to 73bbfdf Compare March 12, 2026 02:01
@davetist
Copy link
Author

davetist commented Mar 12, 2026

It modifies the session id too because apparently in macOS if the name is too long it doesn't work

@davetist
Copy link
Author

Added more context to the PR description

@davetist davetist changed the title Fix browser cleanup consistency and screenshot recovery Improve browser tool cleanup, PATH setup, and screenshot recovery Mar 12, 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.

1 participant