# NousResearch/hermes-agent · Browser tool timeout and concurrency race conditions

## Open now

- 1 issue, 0 PRs open on GitHub.
- #24669: concurrency: four TOCTOU races in browser_tool.py lazy-init singletons

These issues and fixes center on two related problems in browser_tool.py: \`_get_command_timeout\` returning None on certain exception or cleanup paths, and TOCTOU race conditions in lazy-init singletons. Multiple fixes address guarding the timeout cache and closing concurrency windows.

## TL;DR

- 10 member(s): 1 open issue(s), 2 closed issue(s), 0 open PR(s), 1 merged PR(s), 6 PR(s) closed without merging.
- A fix was merged to main in PR #54766. Verify it addresses your case.
- **Recommended:** read PR #54766 (and any competing PRs below) before filing. Confirm the merged change covers your symptom.

## Maintainer resolution verdict

- Internal-tracker verdict over this cluster's issue members: `unresolved`. This is distinct from the fix status above. Weigh it yourself.

## How this cluster was grouped

- Link evidence (`STRUCTURAL_PARTIAL`): some members are connected by structural evidence (shared changed files, or a closing/superseded reference); the rest are joined by an automated relevance judgement over their text.
- Members outside that structural core may belong to a different underlying problem.
- Pairwise link density `0.22`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked.

## Open related issues

- #24669: concurrency: four TOCTOU races in browser_tool.py lazy-init singletons

## Closed / duplicate issues

- #14331: \[Bug\]: browser_navigate crashes with TypeError: '>' not supported between instances of 'int' and 'NoneType' after hermes update
- #14783: \[Bug\]: browser_tool _get_command_timeout crashes with TypeError after cleanup race

## Prior & competing PRs

- **#14346: fix(browser): guard command timeout cache reset**. closed without merge · fix tier `suggested`
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `#14331`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#24772: fix(core): close TOCTOU race windows in lazy module resolvers**. closed without merge · fix tier `suggested`
  - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `#24671`, `#24669`, `#24692`, `#24687`, `#24694`, `#24679`, `#24673`
  - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._
- **#54766: fix(browser_tool): prevent _get_command_timeout from returning None on torn-cache path**. merged to main · fix tier `verified`
  - maintainer direction: category `merged`
  - files: `tests/tools/test_browser_command_timeout_race.py`, `tools/browser_tool.py`
  - fix hunks:
  - `tests/tools/test_browser_command_timeout_race.py`

    ```diff
    @@ -0,0 +1,109 @@
    +"""Regression tests for the _get_command_timeout cache race (#14331).
    +
    +Before the fix:
    +  ``_command_timeout_resolved`` was set to ``True`` *before*
    +  ``_cached_command_timeout`` was assigned. If the body raised between those
    +  two statements (e.g. inside ``read_raw_config``), or a re-entrant/concurrent
    +  reader hit the cache between them, the function returned ``None``. Callers
    +  then evaluated ``max(None, 60)`` and crashed with::
    +
    +      TypeError: '>' not supported between instances of 'int' and 'NoneType'
    +
    +The fix:
    +  1. assign cache before flipping the resolved flag,
    +  2. flip the resolved flag *off* before nulling the cache in
    +     ``cleanup_all_browsers()``,
    +  3. expose ``_safe_command_timeout()`` as defense in depth.
    +"""
    +
    +from unittest.mock import patch
    +
    +
    +class TestGetCommandTimeoutRace:
    +    def setup_method(self):
    +        from tools import browser_tool
    +
    +        self.bt = browser_tool
    +        self._orig_cache = browser_tool._cached_command_timeout
    +        self._orig_resolved = browser_tool._command_timeout_resolved
    +        browser_tool._cached_command_timeout = None
    +        browser_tool._command_timeout_resolved = False
    +
    +    def teardown_method(self):
    +        self.bt._cached_command_timeout = self._orig_cache
    +        self.bt._command_timeout_resolved = self._orig_resolved
    +
    +    def test_returns_default_when_config_read_raises(self):
    +        """If config reading blows up, we still return an int (not None)."""
    +        with patch(
    +            "hermes_cli.config.read_raw_config", side_effect=RuntimeError("boom")
    +        ):
    +            result = self.bt._get_command_timeout()
    +
    +        assert isinstance(result, int)
    +        assert result == self.bt.DEFAULT_COMMAND_TIMEOUT
    +        # Cache must be populated (not left as None) once resolved is True.
    +        assert self.bt._cached_command_timeout is not None
    +        assert self.bt._command_timeout_resolved is True
    +
    +    def test_cache_assigned_before_resolved_flag(self):
    +        """Invariant: if resolved=True then cache must not be None."""
    +        with patch(
    +            "hermes_cli.config.read_raw_config", side_effect=RuntimeError("boom")
    +        ):
    +            self.bt._get_command_timeout()
    +
    +        # The bug was: resolved=True while cache=None. Assert that's impossible.
    +        assert not (
    +            self.bt._command_timeout_resolved
    +            and self.bt._cached_command_timeout is None
    +        )
    +
    +    def test_safe_command_timeout_never_returns_none(self):
    +        """Defense-in-depth helper survives a manually corrupted cache."""
    +        # Simulate the pre-fix bug state directly.
    +        self.bt._command_timeout_resolved = True
    +        self.bt._cached_command_timeout = None
    +
    +        result = self.bt._safe_command_timeout()
    +        assert isinstance(result, int)
    +        assert result == self.bt.DEFAULT_COMMAND_TIMEOUT
    +
    +    def test_safe_command_timeout_preserves_zero(self):
    +        """``or DEFAULT_COMMAND_TIMEOUT`` would swallow a legit 0.
    +
    +        We use ``is not None`` so a configured 0 stays 0. (In practice the
    +        caller floor is 5s, but the helper itself must be honest.)
    +        """
    +        self.bt._command_timeout_resolved = True
    +        self.bt._cached_command_timeout = 0
    +
    +        assert self.bt._safe_command_timeout() == 0
    +
    +    def test_cleanup_resets_flag_before_nulling_cache(self):
    +        """After cleanup, observers must never see resolved=True with cache=None."""
    +        # Warm the cache first.
    +        with patch(
    +            "hermes_cli.config.read_raw_config", side_effect=RuntimeError("boom")
    +        ):
    +            self.bt._get_command_timeout()
    +        assert self.bt._command_timeout_resolved is True
    +
    +        self.bt.cleanup_all_browsers()
    +
    +        # Post-cleanup: both must be reset together; specifically resolved must
    +        # not be True while cache is None (the original race window).
    +        assert not (
    +            self.bt._command_timeout_resolved
    +            and self.bt._cached_command_timeout is None
    +        )
    +
    +    def test_max_call_site_pattern_never_raises(self):
    +        """The exact expression from browser_navigate must not raise TypeError."""
    +        # Force the corrupted state the bug used to produce.
    +        self.bt._command_timeout_resolved = True
    +        self.bt._cached_command_timeout = None
    +
    +        # This is the literal line from browser_navigate() after the fix.
    +        timeout = max(self.bt._safe_command_timeout(), 60)
    +        assert timeout == 60
    ```
  - `tools/browser_tool.py`

    ```diff
    @@ -244,10 +244,9 @@ def _get_command_timeout() -> int:
         cached after the first call and cleared by ``cleanup_all_browsers()``.
         """
         global _cached_command_timeout, _command_timeout_resolved
    -    if _command_timeout_resolved:
    -        return _cached_command_timeout  # type: ignore[return-value]
    +    if _command_timeout_resolved and _cached_command_timeout is not None:
    +        return _cached_command_timeout
     
    -    _command_timeout_resolved = True
         result = DEFAULT_COMMAND_TIMEOUT
         try:
             from hermes_cli.config import read_raw_config
    ```

    ```diff
    @@ -257,13 +256,29 @@ def _get_command_timeout() -> int:
                 result = max(int(val), 5)  # Floor at 5s to avoid instant kills
         except Exception as e:
             logger.debug("Could not read command_timeout from config: %s", e)
    +    # Assign the cached value BEFORE flipping the resolved flag so a
    +    # concurrent reader cannot observe ``resolved=True`` while the cache
    +    # is still ``None`` (see issue #14331).
         _cached_command_timeout = result
    +    _command_timeout_resolved = True
         return result
     
     
    +def _safe_command_timeout() -> int:
    +    """Like ``_get_command_timeout`` but guaranteed non-None.
    +
    +    Defense in depth against the race fixed in ``_get_command_timeout``:
    +    if anything ever returns ``None`` (e.g. cache reset mid-flight), fall
    +    back to ``DEFAULT_COMMAND_TIMEOUT``. Uses ``is not None`` rather than
    +    ``or`` so a legitimately configured ``0`` is preserved.
    +    """
    +    val = _get_command_timeout()
    +    return val if val is not None else DEFAULT_COMMAND_TIMEOUT
    +
    +
     def _get_open_command_timeout(*, first_open: bool = False) -> int:
         """Timeout for agent-browser ``open`` (navigation / daemon cold start)."""
    -    base = _get_command_timeout()
    +    base = _safe_command_timeout()
         floor = MIN_FIRST_OPEN_TIMEOUT if first_open else MIN_OPEN_TIMEOUT
         return max(base, floor)
     
    ```

    ```diff
    @@ -2180,7 +2195,7 @@ def _run_browser_command(
             Parsed JSON response from agent-browser
         """
         if timeout is None:
    -        timeout = _get_command_timeout()
    +        timeout = _safe_command_timeout()
         args = args or []
     
         # Build the command
    ```

    ```diff
    @@ -4029,8 +4044,10 @@ def cleanup_all_browsers() -> None:
         _cached_agent_browser = None
         _agent_browser_resolved = False
         _discover_homebrew_node_dirs.cache_clear()
    -    _cached_command_timeout = None
    +    # Flip the resolved flag BEFORE nulling the cache so a concurrent
    +    # reader never sees ``resolved=True`` with ``cache=None`` (#14331).
         _command_timeout_resolved = False
    +    _cached_command_timeout = None
         _cached_chromium_installed = None
         global _chromium_autoinstall_attempted
         _chromium_autoinstall_attempted = False
    ```
- **#14338: fix(browser_tool): prevent _get_command_timeout from returning None on exception path**. closed without merge
  - maintainer direction: category `merged_via`; implication `superseded`; refs `c79e6bcea`, `#54766`
- **#14779: fix(tools): guard _get_command_timeout against None return after browser cleanup race**. closed without merge
  - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#14331`, `#54766`, `#14338`
- **#14843: fix(browser): guard command timeout cache invalidation**. closed without merge
  - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#14331`, `#54766`, `#14338`
- **#24671: fix(browser_tool): eliminate four TOCTOU races in lazy-init singletons**. closed without merge
  - maintainer direction: category `silent_close`; implication `active`

## Fix status

- Merged to main in PR #54766 (cited above). No release data is tracked. Verify downstream.
- Fix last verified (merged) `2026-06-29`.
- Subsystem: `agent`