feat: add Notte browser provider integration for hermes-agent #758
feat: add Notte browser provider integration for hermes-agent #758
Conversation
Adds complete implementation files for integrating Notte as a cloud browser provider in NousResearch/hermes-agent. Includes provider implementation, unit tests, and a README with step-by-step instructions for the external PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Notte cloud browser provider to hermes-agent: new provider implementation ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
| session_data = response.json() | ||
| notte_session_id = session_data["session_id"] | ||
|
|
||
| # Step 2: Get CDP WebSocket URL | ||
| debug_response = requests.get( | ||
| f"{config['base_url']}/sessions/{notte_session_id}/debug", | ||
| headers=headers, | ||
| timeout=60, | ||
| ) | ||
|
|
||
| if not debug_response.ok: | ||
| # Best-effort cleanup if we can't get the CDP URL | ||
| self.emergency_cleanup(notte_session_id) | ||
| raise RuntimeError( | ||
| f"Failed to get Notte session CDP URL: " | ||
| f"{debug_response.status_code} {debug_response.text}" | ||
| ) | ||
|
|
||
| debug_data = debug_response.json() | ||
| cdp_url = debug_data["ws"]["cdp"] |
There was a problem hiding this comment.
Unguarded
KeyError on API response fields
Both session_data["session_id"] (line 106) and debug_data["ws"]["cdp"] (line 124) will raise a bare KeyError if the API returns an unexpected shape (e.g. an error body that is valid JSON but missing those keys). This bypasses the RuntimeError-based error reporting and leaks confusing tracebacks to callers. The response.ok check only guards against non-2xx status codes — a 200 with an unexpected payload still reaches these lines.
session_data = response.json()
notte_session_id = session_data.get("session_id")
if not notte_session_id:
raise RuntimeError(
f"Notte API returned unexpected response (missing 'session_id'): {response.text}"
)
# ... after debug_response.ok check ...
debug_data = debug_response.json()
cdp_url = debug_data.get("ws", {}).get("cdp")
if not cdp_url:
self.emergency_cleanup(notte_session_id)
raise RuntimeError(
f"Notte API returned unexpected debug response (missing 'ws.cdp'): {debug_response.text}"
)Prompt To Fix With AI
This is a comment left during a code review.
Path: integrations/hermes-agent/tools/browser_providers/notte.py
Line: 105-124
Comment:
**Unguarded `KeyError` on API response fields**
Both `session_data["session_id"]` (line 106) and `debug_data["ws"]["cdp"]` (line 124) will raise a bare `KeyError` if the API returns an unexpected shape (e.g. an error body that is valid JSON but missing those keys). This bypasses the `RuntimeError`-based error reporting and leaks confusing tracebacks to callers. The `response.ok` check only guards against non-2xx status codes — a 200 with an unexpected payload still reaches these lines.
```python
session_data = response.json()
notte_session_id = session_data.get("session_id")
if not notte_session_id:
raise RuntimeError(
f"Notte API returned unexpected response (missing 'session_id'): {response.text}"
)
# ... after debug_response.ok check ...
debug_data = debug_response.json()
cdp_url = debug_data.get("ws", {}).get("cdp")
if not cdp_url:
self.emergency_cleanup(notte_session_id)
raise RuntimeError(
f"Notte API returned unexpected debug response (missing 'ws.cdp'): {debug_response.text}"
)
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| enable_proxies = os.environ.get("NOTTE_PROXIES", "true").lower() != "false" | ||
| enable_captchas = os.environ.get("NOTTE_SOLVE_CAPTCHAS", "false").lower() == "true" | ||
| timeout_minutes = int(os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))) |
There was a problem hiding this comment.
int() conversion without error handling
If NOTTE_TIMEOUT_MINUTES is set to a non-integer string (e.g. "15m" or "auto"), int() will raise a ValueError with no context about which env var caused the failure. A small guard improves the user-facing error:
_raw_timeout = os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))
try:
timeout_minutes = int(_raw_timeout)
except ValueError:
raise ValueError(
f"NOTTE_TIMEOUT_MINUTES must be an integer, got: {_raw_timeout!r}"
)Prompt To Fix With AI
This is a comment left during a code review.
Path: integrations/hermes-agent/tools/browser_providers/notte.py
Line: 73
Comment:
**`int()` conversion without error handling**
If `NOTTE_TIMEOUT_MINUTES` is set to a non-integer string (e.g. `"15m"` or `"auto"`), `int()` will raise a `ValueError` with no context about which env var caused the failure. A small guard improves the user-facing error:
```python
_raw_timeout = os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))
try:
timeout_minutes = int(_raw_timeout)
except ValueError:
raise ValueError(
f"NOTTE_TIMEOUT_MINUTES must be an integer, got: {_raw_timeout!r}"
)
```
How can I resolve this? If you propose a fix, please make it concise.| features_enabled: Dict[str, object] = { | ||
| "proxies": enable_proxies, | ||
| "solve_captchas": enable_captchas, | ||
| "stealth": True, | ||
| } |
There was a problem hiding this comment.
stealth: True advertised in features but never sent to the API
features_enabled["stealth"] is hardcoded to True and returned to the caller, but stealth is never added to payload (unlike proxies and solve_captchas). If stealth is on by default server-side this is harmless, but it's misleading — callers inspecting result["features"]["stealth"] will assume it was explicitly requested. Either add payload["stealth"] = True to make it explicit, or remove it from features_enabled if it's unconditionally applied.
Prompt To Fix With AI
This is a comment left during a code review.
Path: integrations/hermes-agent/tools/browser_providers/notte.py
Line: 84-88
Comment:
**`stealth: True` advertised in features but never sent to the API**
`features_enabled["stealth"]` is hardcoded to `True` and returned to the caller, but `stealth` is never added to `payload` (unlike `proxies` and `solve_captchas`). If stealth is on by default server-side this is harmless, but it's misleading — callers inspecting `result["features"]["stealth"]` will assume it was explicitly requested. Either add `payload["stealth"] = True` to make it explicit, or remove it from `features_enabled` if it's unconditionally applied.
How can I resolve this? If you propose a fix, please make it concise.| class TestNotteProviderRegistry: | ||
| def test_notte_in_provider_registry(self): | ||
| from tools.browser_tool import _PROVIDER_REGISTRY | ||
| from tools.browser_providers.notte import NotteProvider | ||
|
|
||
| assert "notte" in _PROVIDER_REGISTRY | ||
| assert _PROVIDER_REGISTRY["notte"] is NotteProvider |
There was a problem hiding this comment.
Registry test will fail until
browser_tool.py is patched
TestNotteProviderRegistry imports _PROVIDER_REGISTRY from tools.browser_tool and asserts that "notte" is registered there. However, the required change to browser_tool.py is only described in the README instructions for the external PR — it is not applied in this repo. Running the test suite as-is (e.g. pytest tests/tools/test_notte_provider.py) will fail on this test class with an AssertionError. Either mock/skip this test, or note prominently that this test must be run only after the registry patch is applied to hermes-agent.
Prompt To Fix With AI
This is a comment left during a code review.
Path: integrations/hermes-agent/tests/tools/test_notte_provider.py
Line: 283-289
Comment:
**Registry test will fail until `browser_tool.py` is patched**
`TestNotteProviderRegistry` imports `_PROVIDER_REGISTRY` from `tools.browser_tool` and asserts that `"notte"` is registered there. However, the required change to `browser_tool.py` is only described in the README instructions for the external PR — it is not applied in this repo. Running the test suite as-is (e.g. `pytest tests/tools/test_notte_provider.py`) will fail on this test class with an `AssertionError`. Either mock/skip this test, or note prominently that this test must be run only after the registry patch is applied to hermes-agent.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
integrations/hermes-agent/tests/tools/test_notte_provider.py (1)
130-161: Add a negative test for malformedNOTTE_TIMEOUT_MINUTES.This suite validates the happy-path timeout value, but it doesn’t cover invalid env input (e.g.,
"abc"). Add one test so timeout parsing behavior is pinned.Suggested test addition
+ def test_create_session_raises_on_invalid_timeout_minutes(self, monkeypatch): + monkeypatch.setenv("NOTTE_API_KEY", "nk-test") + monkeypatch.setenv("NOTTE_TIMEOUT_MINUTES", "abc") + from tools.browser_providers.notte import NotteProvider + + with pytest.raises(ValueError, match="NOTTE_TIMEOUT_MINUTES"): + NotteProvider().create_session("task_bad_timeout")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tests/tools/test_notte_provider.py` around lines 130 - 161, Add a new test (e.g., test_create_session_handles_malformed_timeout) that sets monkeypatch env NOTTE_TIMEOUT_MINUTES="abc" and other env knobs as needed, patches tools.browser_providers.notte.requests similar to existing tests, calls NotteProvider().create_session("task_malformed"), and asserts that no exception is raised and the outgoing post payload (mock_requests.post.call_args[1]["json"]) contains a "timeout_minutes" key whose value is an int (i.e., the provider normalized/handled the malformed string); also assert result["features"] keys (like "proxies" and "solve_captchas") still exist to pin overall behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations/hermes-agent/README.md`:
- Around line 38-40: The unlabeled fenced code blocks in the README (the block
containing "- NOTTE_API_KEY: API key for Notte cloud mode
(https://console.notte.cc)" and the larger block starting with "## Summary")
trigger markdownlint MD040; update those triple-backtick fences to include a
language identifier (e.g., ```text for the single-line env note and ```md for
the Markdown summary block) so the code fences are labeled, and re-run
markdownlint to verify the MD040 warning is resolved.
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Line 73: The assignment to timeout_minutes uses
int(os.environ.get("NOTTE_TIMEOUT_MINUTES", ...)) and will raise ValueError for
non-numeric env values; change it to validate/parse the environment variable
first (e.g., read env_val = os.environ.get("NOTTE_TIMEOUT_MINUTES"), if env_val
is not None try to int() inside a try/except ValueError and fall back to
_DEFAULT_TIMEOUT_MINUTES) and optionally log or warn when parsing fails so
session creation uses a safe default instead of crashing; update the
timeout_minutes initialization in notte.py accordingly.
- Around line 92-121: The create_session method currently issues requests.post
and requests.get without exception handling, which can leave a server-created
session uncleaned if a network error occurs; wrap the POST and subsequent GET in
try/except blocks that catch requests.RequestException (and other unexpected
exceptions), ensure that if the POST succeeds and produces notte_session_id you
always call self.emergency_cleanup(notte_session_id) on any error during the GET
or in the exception path, and re-raise or convert the exception to a
RuntimeError with context; specifically update create_session to set
notte_session_id only after a successful POST response.json(), use a try/finally
or nested try/except around the debug_response GET to guarantee cleanup via
emergency_cleanup(notte_session_id) if debug retrieval fails or raises, and
include the original exception details in the raised error messages.
---
Nitpick comments:
In `@integrations/hermes-agent/tests/tools/test_notte_provider.py`:
- Around line 130-161: Add a new test (e.g.,
test_create_session_handles_malformed_timeout) that sets monkeypatch env
NOTTE_TIMEOUT_MINUTES="abc" and other env knobs as needed, patches
tools.browser_providers.notte.requests similar to existing tests, calls
NotteProvider().create_session("task_malformed"), and asserts that no exception
is raised and the outgoing post payload
(mock_requests.post.call_args[1]["json"]) contains a "timeout_minutes" key whose
value is an int (i.e., the provider normalized/handled the malformed string);
also assert result["features"] keys (like "proxies" and "solve_captchas") still
exist to pin overall behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7d77729-b900-49e9-9132-99a21cc1afef
📒 Files selected for processing (3)
integrations/hermes-agent/README.mdintegrations/hermes-agent/tests/tools/test_notte_provider.pyintegrations/hermes-agent/tools/browser_providers/notte.py
|
|
||
| enable_proxies = os.environ.get("NOTTE_PROXIES", "true").lower() != "false" | ||
| enable_captchas = os.environ.get("NOTTE_SOLVE_CAPTCHAS", "false").lower() == "true" | ||
| timeout_minutes = int(os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))) |
There was a problem hiding this comment.
Validate NOTTE_TIMEOUT_MINUTES before casting.
Line 73 does a direct int(...) cast. A non-numeric env value crashes session creation with an uncaught ValueError.
Suggested patch
- timeout_minutes = int(os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES)))
+ timeout_raw = os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))
+ try:
+ timeout_minutes = int(timeout_raw)
+ if timeout_minutes <= 0:
+ raise ValueError
+ except ValueError as e:
+ raise ValueError("NOTTE_TIMEOUT_MINUTES must be a positive integer") from e📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| timeout_minutes = int(os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))) | |
| timeout_raw = os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES)) | |
| try: | |
| timeout_minutes = int(timeout_raw) | |
| if timeout_minutes <= 0: | |
| raise ValueError | |
| except ValueError as e: | |
| raise ValueError("NOTTE_TIMEOUT_MINUTES must be a positive integer") from e |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integrations/hermes-agent/tools/browser_providers/notte.py` at line 73, The
assignment to timeout_minutes uses int(os.environ.get("NOTTE_TIMEOUT_MINUTES",
...)) and will raise ValueError for non-numeric env values; change it to
validate/parse the environment variable first (e.g., read env_val =
os.environ.get("NOTTE_TIMEOUT_MINUTES"), if env_val is not None try to int()
inside a try/except ValueError and fall back to _DEFAULT_TIMEOUT_MINUTES) and
optionally log or warn when parsing fails so session creation uses a safe
default instead of crashing; update the timeout_minutes initialization in
notte.py accordingly.
…f sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…f sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
integrations/hermes-agent/tools/browser_providers/notte.py (3)
93-125:⚠️ Potential issue | 🟠 MajorGuard HTTP/JSON failure paths and guarantee cleanup after session start.
Lines 93-125 can throw
requests/JSON/key errors, and cleanup is not guaranteed on all debug-fetch failure modes. This can leak remote sessions and surface uncontrolled exceptions.Suggested fix
- response = requests.post( - f"{config['base_url']}/sessions/start", - headers=headers, - json=payload, - timeout=60, - ) - - if not response.ok: - raise RuntimeError( - f"Failed to create Notte session: " - f"{response.status_code} {response.text}" - ) - - session_data = response.json() - notte_session_id = session_data["session_id"] + try: + response = requests.post( + f"{config['base_url']}/sessions/start", + headers=headers, + json=payload, + timeout=60, + ) + if not response.ok: + raise RuntimeError( + f"Failed to create Notte session: " + f"{response.status_code} {response.text}" + ) + session_data = response.json() + notte_session_id = session_data["session_id"] + except (requests.RequestException, KeyError, TypeError, ValueError) as e: + raise RuntimeError("Failed to create Notte session") from e # Step 2: Get CDP WebSocket URL - debug_response = requests.get( - f"{config['base_url']}/sessions/{notte_session_id}/debug", - headers=headers, - timeout=60, - ) - - if not debug_response.ok: - # Best-effort cleanup if we can't get the CDP URL - self.emergency_cleanup(notte_session_id) - raise RuntimeError( - f"Failed to get Notte session CDP URL: " - f"{debug_response.status_code} {debug_response.text}" - ) - - debug_data = debug_response.json() - cdp_url = debug_data["ws"]["cdp"] + try: + debug_response = requests.get( + f"{config['base_url']}/sessions/{notte_session_id}/debug", + headers=headers, + timeout=60, + ) + if not debug_response.ok: + raise RuntimeError( + f"Failed to get Notte session CDP URL: " + f"{debug_response.status_code} {debug_response.text}" + ) + debug_data = debug_response.json() + cdp_url = debug_data["ws"]["cdp"] + except (requests.RequestException, KeyError, TypeError, ValueError) as e: + self.emergency_cleanup(notte_session_id) + raise RuntimeError("Failed to get Notte session CDP URL") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 93 - 125, Wrap the Notte session start and debug-fetch sequence (the requests.post to "/sessions/start", response.json usage, requests.get to "/sessions/{notte_session_id}/debug", debug_response.json and cdp_url extraction) in robust try/except/finally handling: catch requests.RequestException, ValueError/JSONDecodeError and KeyError from both JSON parses and key lookups, ensure that if notte_session_id was created (set) you always call self.emergency_cleanup(notte_session_id) in a finally block on error paths, and raise a clear RuntimeError with the original exception details when failing; keep the existing RuntimeError messages for HTTP non-ok responses but also guard json() and key access to avoid leaking sessions.
143-170:⚠️ Potential issue | 🟠 MajorAvoid logging raw session IDs in warning/error paths.
Lines 143/157/164/170 log full session IDs; treat these as sensitive operational identifiers and redact/hash consistently (as already done at Line 179).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 143 - 170, The logs in the session-close and emergency paths currently print raw session_id; update all logger.warning/error/debug calls in the session shutdown method (the block that calls requests.delete and the surrounding try/except) and in emergency_cleanup to pass a redacted or hashed form of session_id instead of the raw value (use the same redaction/hash helper already used at Line 179 to ensure consistent masking). Replace occurrences where session_id is interpolated into messages (e.g., "Cannot close Notte session %s", "Failed to close Notte session %s", "Exception closing Notte session %s", and the emergency_cleanup warning) to use the helper output. Ensure the helper is imported/available in this module and apply it to every log call in these functions so identifiers are consistently protected.
74-74:⚠️ Potential issue | 🟡 MinorHarden
NOTTE_TIMEOUT_MINUTESparsing and error message.Line 74 can raise a raw
ValueErrorwith no config context; this is brittle during deployment misconfiguration.Suggested fix
- timeout_minutes = int(os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))) + timeout_raw = os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES)) + try: + timeout_minutes = int(timeout_raw) + if timeout_minutes <= 0: + raise ValueError + except ValueError as e: + raise ValueError("NOTTE_TIMEOUT_MINUTES must be a positive integer") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` at line 74, The direct int() conversion of NOTTE_TIMEOUT_MINUTES can raise an uninformative ValueError; update the logic around timeout_minutes (currently using os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))) to validate and parse the environment variable in a try/except block that catches ValueError, logs or raises a new error with clear config context mentioning NOTTE_TIMEOUT_MINUTES and the fallback _DEFAULT_TIMEOUT_MINUTES, and uses the fallback when appropriate; reference the timeout_minutes variable and the NOTTE_TIMEOUT_MINUTES env lookup so the fix is applied where the variable is parsed.
🧹 Nitpick comments (2)
integrations/hermes-agent/tools/browser_providers/notte.py (2)
76-89: Align advertisedstealthcapability with the request payload.Line 88 reports
stealth=True, but Lines 76-84 never send a stealth flag to/sessions/start. Either send it explicitly or avoid claiming it as enabled in returned features.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 76 - 89, The features_enabled dict claims "stealth": True but the request payload (payload in the same function) never includes a stealth flag; update the code so the advertised capability matches the request by adding the stealth flag to the payload when appropriate (e.g., set payload["stealth"] = True when you want stealth enabled, using the same condition logic or a new enable_stealth variable), or alternatively remove/false the "stealth" entry from features_enabled; adjust the payload and features_enabled variables (payload and features_enabled) consistently so the /sessions/start request and the reported capabilities match.
163-165: Narrow broad exception handlers to request-layer failures.Catching
Exceptionhere can hide programming errors; preferrequests.RequestException(and only widen intentionally).Suggested fix
- except Exception as e: + except requests.RequestException as e: logger.error("Exception closing Notte session %s: %s", session_id, e) return False ... - except Exception as e: + except requests.RequestException as e: session_id_hash = hashlib.sha256(session_id.encode("utf-8")).hexdigest()[:8] logger.debug( "Emergency cleanup failed for Notte session (id hash %s): %s", session_id_hash, e, )Also applies to: 178-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 163 - 165, Replace the overly broad "except Exception as e" in the Notte session close/cleanup logic with a request-layer-only handler: import requests and catch "except requests.RequestException as e" in the block that logs "Exception closing Notte session %s: %s" (and the similar handler later around the other occurrence), so that programming errors are not swallowed; if you need to preserve the original behavior for non-request errors, re-raise other exceptions after handling RequestException.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 3-10: The import block in notte.py is unsorted and triggers Ruff
I001; reorder imports into PEP8 groups (standard library, third-party, local)
and alphabetize within each group: put hashlib, logging, os, uuid, typing (Any,
Dict, Optional) first, then third-party requests, and remove any
blank-line/order issues so the top-of-file imports match Ruff's expected order;
after reordering, run ruff/format to confirm the I001 warning is resolved.
---
Duplicate comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 93-125: Wrap the Notte session start and debug-fetch sequence (the
requests.post to "/sessions/start", response.json usage, requests.get to
"/sessions/{notte_session_id}/debug", debug_response.json and cdp_url
extraction) in robust try/except/finally handling: catch
requests.RequestException, ValueError/JSONDecodeError and KeyError from both
JSON parses and key lookups, ensure that if notte_session_id was created (set)
you always call self.emergency_cleanup(notte_session_id) in a finally block on
error paths, and raise a clear RuntimeError with the original exception details
when failing; keep the existing RuntimeError messages for HTTP non-ok responses
but also guard json() and key access to avoid leaking sessions.
- Around line 143-170: The logs in the session-close and emergency paths
currently print raw session_id; update all logger.warning/error/debug calls in
the session shutdown method (the block that calls requests.delete and the
surrounding try/except) and in emergency_cleanup to pass a redacted or hashed
form of session_id instead of the raw value (use the same redaction/hash helper
already used at Line 179 to ensure consistent masking). Replace occurrences
where session_id is interpolated into messages (e.g., "Cannot close Notte
session %s", "Failed to close Notte session %s", "Exception closing Notte
session %s", and the emergency_cleanup warning) to use the helper output. Ensure
the helper is imported/available in this module and apply it to every log call
in these functions so identifiers are consistently protected.
- Line 74: The direct int() conversion of NOTTE_TIMEOUT_MINUTES can raise an
uninformative ValueError; update the logic around timeout_minutes (currently
using os.environ.get("NOTTE_TIMEOUT_MINUTES", str(_DEFAULT_TIMEOUT_MINUTES))) to
validate and parse the environment variable in a try/except block that catches
ValueError, logs or raises a new error with clear config context mentioning
NOTTE_TIMEOUT_MINUTES and the fallback _DEFAULT_TIMEOUT_MINUTES, and uses the
fallback when appropriate; reference the timeout_minutes variable and the
NOTTE_TIMEOUT_MINUTES env lookup so the fix is applied where the variable is
parsed.
---
Nitpick comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 76-89: The features_enabled dict claims "stealth": True but the
request payload (payload in the same function) never includes a stealth flag;
update the code so the advertised capability matches the request by adding the
stealth flag to the payload when appropriate (e.g., set payload["stealth"] =
True when you want stealth enabled, using the same condition logic or a new
enable_stealth variable), or alternatively remove/false the "stealth" entry from
features_enabled; adjust the payload and features_enabled variables (payload and
features_enabled) consistently so the /sessions/start request and the reported
capabilities match.
- Around line 163-165: Replace the overly broad "except Exception as e" in the
Notte session close/cleanup logic with a request-layer-only handler: import
requests and catch "except requests.RequestException as e" in the block that
logs "Exception closing Notte session %s: %s" (and the similar handler later
around the other occurrence), so that programming errors are not swallowed; if
you need to preserve the original behavior for non-request errors, re-raise
other exceptions after handling RequestException.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31ae8032-04e8-4385-9756-4ecf46d4b448
📒 Files selected for processing (1)
integrations/hermes-agent/tools/browser_providers/notte.py
- Guard against unexpected API response shapes with .get() and explicit error messages instead of bare KeyError - Validate NOTTE_TIMEOUT_MINUTES with clear ValueError on bad input - Remove misleading stealth: True from features (server-side default) - Wrap create_session in try/except to always attempt cleanup on failure - Skip registry test that requires hermes-agent patches applied - Add edge case tests: missing session_id, missing ws.cdp, network error cleanup, invalid timeout - Fix markdown lint MD040 (unlabeled code fences) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
integrations/hermes-agent/tools/browser_providers/notte.py (2)
3-10:⚠️ Potential issue | 🟡 MinorFix import ordering to unblock Ruff I001.
The import block is not Ruff-sorted, which is currently failing CI.
Suggested fix
-import logging -import os -import uuid -from typing import Any, Dict, Optional import hashlib +import logging +import os +import uuid +from typing import Any, Dict, Optional🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 3 - 10, Reorder the imports in notte.py to satisfy Ruff I001: group standard library imports (hashlib, logging, os, uuid, typing) first in alphabetical order, then a blank line, then third-party imports (requests) and any local imports last; ensure the typing imports remain as "from typing import Any, Dict, Optional" and place "import hashlib" with the other stdlib imports so the import block is correctly sorted and separated by one blank line between stdlib and third-party sections.
163-190:⚠️ Potential issue | 🟠 MajorAvoid logging raw Notte session IDs.
Lines 163-190 log plaintext
session_id. Treat session IDs as sensitive operational tokens and log only a redacted/hash form consistently (you already do this in Line 199 for one path).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 163 - 190, The code logs raw session_id in the Notte session-close path and in emergency_cleanup; replace all uses of session_id in logger calls inside the session stop flow (the requests.delete block and its warning/error logs) and inside def emergency_cleanup(self, session_id: str) with the same redacted/hashed representation used elsewhere in this module (the existing redaction helper used at the other Notte path on line 199), so call that helper on session_id before passing it to logger.debug/warning/error to ensure consistent, non-plaintext logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations/hermes-agent/tests/tools/test_notte_provider.py`:
- Around line 287-289: The two local imports in the skipped registry test are
out of alphabetical order (from tools.browser_tool import _PROVIDER_REGISTRY and
from tools.browser_providers.notte import NotteProvider); reorder them so the
import names are sorted (i.e., import NotteProvider before _PROVIDER_REGISTRY or
follow your project's import-sorting convention) to satisfy Ruff I001 and keep
the function-scoped import block consistent.
- Line 327: The test's pytest.raises call currently uses match="missing
'ws.cdp'" where the dot is treated as a regex wildcard; update the assertion to
use a raw string with the dot escaped (e.g., r"missing 'ws\.cdp'") in the
pytest.raises(..., match=...) argument so the message matches the literal
"ws.cdp" instead of any character.
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 75-81: The code parses NOTTE_TIMEOUT_MINUTES into raw_timeout and
timeout_minutes but doesn't reject zero or negative values; after converting
raw_timeout to int (the timeout_minutes variable in the
integrations/hermes-agent/tools/browser_providers/notte.py module), add a
validation that timeout_minutes > 0 and raise a ValueError with a clear message
(e.g. "NOTTE_TIMEOUT_MINUTES must be a positive integer, got: {raw_timeout!r}")
if the check fails; keep using _DEFAULT_TIMEOUT_MINUTES as the fallback for
missing env var.
- Around line 69-157: Change the loose return annotation of create_session to a
concrete TypedDict: define a NotteSession TypedDict (import TypedDict and
Optional from typing) with keys session_name: str, bb_session_id: Optional[str],
cdp_url: str, and features: Dict[str, bool] (or a nested Features TypedDict with
proxies: bool, solve_captchas: bool), then update the create_session signature
to return NotteSession and ensure the returned dict literal matches those types
(use Optional for notte_session_id). Reference create_session, features_enabled,
notte_session_id, session_name and cdp_url when applying the change.
---
Duplicate comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 3-10: Reorder the imports in notte.py to satisfy Ruff I001: group
standard library imports (hashlib, logging, os, uuid, typing) first in
alphabetical order, then a blank line, then third-party imports (requests) and
any local imports last; ensure the typing imports remain as "from typing import
Any, Dict, Optional" and place "import hashlib" with the other stdlib imports so
the import block is correctly sorted and separated by one blank line between
stdlib and third-party sections.
- Around line 163-190: The code logs raw session_id in the Notte session-close
path and in emergency_cleanup; replace all uses of session_id in logger calls
inside the session stop flow (the requests.delete block and its warning/error
logs) and inside def emergency_cleanup(self, session_id: str) with the same
redacted/hashed representation used elsewhere in this module (the existing
redaction helper used at the other Notte path on line 199), so call that helper
on session_id before passing it to logger.debug/warning/error to ensure
consistent, non-plaintext logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eef004e1-bfb3-4697-b2c2-172d6075ebf1
📒 Files selected for processing (3)
integrations/hermes-agent/README.mdintegrations/hermes-agent/tests/tools/test_notte_provider.pyintegrations/hermes-agent/tools/browser_providers/notte.py
✅ Files skipped from review due to trivial changes (1)
- integrations/hermes-agent/README.md
- Add TypedDict (NotteSession, NotteFeatures) for create_session return type - Validate NOTTE_TIMEOUT_MINUTES rejects zero and negative values - Fix import order for Ruff I001 compliance - Escape regex dot in pytest.raises match for 'ws.cdp' - Add tests for zero and negative timeout values Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
integrations/hermes-agent/tools/browser_providers/notte.py (1)
206-207:⚠️ Potential issue | 🟠 MajorAvoid logging raw session IDs in warning logs.
This path still logs the plaintext session identifier; hash or redact it consistently.
Proposed redaction patch
- if config is None: - logger.warning("Cannot emergency-cleanup Notte session %s — missing credentials", session_id) + if config is None: + session_id_hash = hashlib.sha256(session_id.encode("utf-8")).hexdigest()[:8] + logger.warning( + "Cannot emergency-cleanup Notte session (id hash %s) — missing credentials", + session_id_hash, + ) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 206 - 207, The warning currently logs the raw session_id in the Notte emergency-cleanup path (logger.warning("Cannot emergency-cleanup Notte session %s — missing credentials", session_id)); change this to redact or hash the session identifier before logging (e.g., compute a short HMAC/sha256 or mask all but last N chars) and log the redacted value instead of session_id so plaintext IDs are never emitted; update the call site in the Notte cleanup function where logger.warning is invoked to use the redacted_session_id variable.integrations/hermes-agent/tests/tools/test_notte_provider.py (1)
285-289:⚠️ Potential issue | 🔴 CriticalSkipped registry test still hard-fails static type checking.
Even with
@pytest.mark.skip, this direct import is analyzed and fails CI (reportMissingImports).Proposed fix (preserve behavior, avoid unresolved import)
`@pytest.mark.skip`(reason="Requires browser_tool.py registry patch applied in hermes-agent") def test_notte_in_provider_registry(self): from tools.browser_providers.notte import NotteProvider - from tools.browser_tool import _PROVIDER_REGISTRY + browser_tool = pytest.importorskip("tools.browser_tool") + provider_registry = browser_tool._PROVIDER_REGISTRY - assert "notte" in _PROVIDER_REGISTRY - assert _PROVIDER_REGISTRY["notte"] is NotteProvider + assert "notte" in provider_registry + assert provider_registry["notte"] is NotteProvider#!/bin/bash # Verify whether direct unresolved import still exists in registry test. rg -n -C2 'from tools\.browser_tool import _PROVIDER_REGISTRY|pytest\.importorskip\("tools\.browser_tool"\)' integrations/hermes-agent/tests/tools/test_notte_provider.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tests/tools/test_notte_provider.py` around lines 285 - 289, The failing static analysis comes from top-level direct imports; change test_notte_in_provider_registry to avoid static imports by importing at runtime via importlib.import_module: inside test_notte_in_provider_registry call pytest.importorskip("tools.browser_tool") then use importlib.import_module("tools.browser_tool") to get _PROVIDER_REGISTRY and importlib.import_module("tools.browser_providers.notte") to get NotteProvider; this keeps the test behavior (still skippable) but prevents the static checker from trying to resolve the direct from ... import statements (replace usages of NotteProvider and _PROVIDER_REGISTRY with the objects retrieved from importlib).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 131-133: The response fields must be validated as strings: update
the checks around notte_session_id (from session_data.get("session_id")) and the
cdp URL (ws.cdp) to use isinstance(..., str) rather than a truthy check; if
either is missing or not a str, raise a RuntimeError with a clear message
identifying which field is invalid (e.g., "invalid or missing session_id" /
"invalid or missing ws.cdp"). Ensure these checks enforce the NotteSession
TypedDict contract (bb_session_id: str, cdp_url: str) and keep the rest of the
code flow unchanged.
---
Duplicate comments:
In `@integrations/hermes-agent/tests/tools/test_notte_provider.py`:
- Around line 285-289: The failing static analysis comes from top-level direct
imports; change test_notte_in_provider_registry to avoid static imports by
importing at runtime via importlib.import_module: inside
test_notte_in_provider_registry call pytest.importorskip("tools.browser_tool")
then use importlib.import_module("tools.browser_tool") to get _PROVIDER_REGISTRY
and importlib.import_module("tools.browser_providers.notte") to get
NotteProvider; this keeps the test behavior (still skippable) but prevents the
static checker from trying to resolve the direct from ... import statements
(replace usages of NotteProvider and _PROVIDER_REGISTRY with the objects
retrieved from importlib).
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 206-207: The warning currently logs the raw session_id in the
Notte emergency-cleanup path (logger.warning("Cannot emergency-cleanup Notte
session %s — missing credentials", session_id)); change this to redact or hash
the session identifier before logging (e.g., compute a short HMAC/sha256 or mask
all but last N chars) and log the redacted value instead of session_id so
plaintext IDs are never emitted; update the call site in the Notte cleanup
function where logger.warning is invoked to use the redacted_session_id
variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 863e01f2-1669-4e99-addc-a6cda58a98cb
📒 Files selected for processing (2)
integrations/hermes-agent/tests/tools/test_notte_provider.pyintegrations/hermes-agent/tools/browser_providers/notte.py
Ensures session_id and ws.cdp are validated as strings, not just truthy, to enforce the NotteSession TypedDict contract. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
integrations/hermes-agent/tools/browser_providers/notte.py (1)
203-220:⚠️ Potential issue | 🟠 MajorHash session_id at line 206 to match the pattern at line 215.
Line 206 logs
session_idin plain text when credentials are missing, but line 215 properly hashes it. Apply the same hashing pattern consistently.🔒 Proposed fix
def emergency_cleanup(self, session_id: str) -> None: config = self._get_config_or_none() if config is None: - logger.warning("Cannot emergency-cleanup Notte session %s — missing credentials", session_id) + session_id_hash = hashlib.sha256(session_id.encode("utf-8")).hexdigest()[:8] + logger.warning("Cannot emergency-cleanup Notte session (id hash %s) — missing credentials", session_id_hash) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 203 - 220, In emergency_cleanup, don't log the raw session_id when config is missing; follow the same hashing used later (session_id_hash = hashlib.sha256(session_id.encode("utf-8")).hexdigest()[:8]) and use that hash in the warning message instead of session_id so both the warning in emergency_cleanup and the debug log (same function) consistently redact the session identifier.
🧹 Nitpick comments (2)
integrations/hermes-agent/tools/browser_providers/notte.py (2)
165-166: Consider handling the empty features case for clearer logs.When no features are enabled, the log message will show "with features: " followed by an empty string. This could be more informative.
✨ Suggested enhancement
- feature_str = ", ".join(k for k, v in features_enabled.items() if v) - logger.info("Created Notte session %s with features: %s", session_name, feature_str) + feature_str = ", ".join(k for k, v in features_enabled.items() if v) or "none" + logger.info("Created Notte session %s with features: %s", session_name, feature_str)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 165 - 166, The log currently prints an empty string when no features are enabled; update the feature string logic used by logger.info so it shows a clear default (e.g., "none" or "no features enabled") instead of blank: change how feature_str is computed from features_enabled (and/or adjust the logger.info call) so that if ", ".join(k for k, v in features_enabled.items() if v) yields an empty string you substitute a descriptive default before calling logger.info("Created Notte session %s with features: %s", session_name, feature_str).
87-97: Chain the exception for better debugging context.The
ValueErrorraised at line 91 should chain from the original exception to preserve the stack trace. This helps with debugging.♻️ Suggested fix
try: timeout_minutes = int(raw_timeout) - except ValueError: + except ValueError as e: raise ValueError( f"NOTTE_TIMEOUT_MINUTES must be a positive integer, got: {raw_timeout!r}" - ) + ) from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/hermes-agent/tools/browser_providers/notte.py` around lines 87 - 97, The ValueError raised when int(raw_timeout) fails should chain the original exception for better traceback: catch the ValueError as a variable (e.g., "except ValueError as e:") and re-raise the new ValueError(f"NOTTE_TIMEOUT_MINUTES must be a positive integer, got: {raw_timeout!r}") from e so the original exception is preserved; update the except block that handles conversion of raw_timeout to timeout_minutes in notte.py accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 175-201: close_session currently logs session_id in plain text in
its logger.debug, logger.warning and logger.error calls; update close_session to
hash or redact the session_id using the same hashing/redaction approach used in
emergency_cleanup before including it in any log message so logs never contain
raw session identifiers — replace session_id with the hashed value in the
debug/warning/error calls inside close_session and call the same helper (the
hashing/redaction implementation used by emergency_cleanup) to ensure consistent
treatment across both methods.
---
Duplicate comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 203-220: In emergency_cleanup, don't log the raw session_id when
config is missing; follow the same hashing used later (session_id_hash =
hashlib.sha256(session_id.encode("utf-8")).hexdigest()[:8]) and use that hash in
the warning message instead of session_id so both the warning in
emergency_cleanup and the debug log (same function) consistently redact the
session identifier.
---
Nitpick comments:
In `@integrations/hermes-agent/tools/browser_providers/notte.py`:
- Around line 165-166: The log currently prints an empty string when no features
are enabled; update the feature string logic used by logger.info so it shows a
clear default (e.g., "none" or "no features enabled") instead of blank: change
how feature_str is computed from features_enabled (and/or adjust the logger.info
call) so that if ", ".join(k for k, v in features_enabled.items() if v) yields
an empty string you substitute a descriptive default before calling
logger.info("Created Notte session %s with features: %s", session_name,
feature_str).
- Around line 87-97: The ValueError raised when int(raw_timeout) fails should
chain the original exception for better traceback: catch the ValueError as a
variable (e.g., "except ValueError as e:") and re-raise the new
ValueError(f"NOTTE_TIMEOUT_MINUTES must be a positive integer, got:
{raw_timeout!r}") from e so the original exception is preserved; update the
except block that handles conversion of raw_timeout to timeout_minutes in
notte.py accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80a96f93-89bc-40b5-bfc6-ac9068660d8d
📒 Files selected for processing (1)
integrations/hermes-agent/tools/browser_providers/notte.py
Extract _hash_id() helper and use it in create_session, close_session, and emergency_cleanup so raw session IDs never appear in logs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…f sensitive information' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Summary
What's included:
Why:
Hermes Agent's onboarding wizard lets users choose a cloud browser provider (Browserbase, Browser Use, Firecrawl, Camofox). Getting Notte listed there puts us in front of 30k+ users. Zero new dependencies + we have browserarena.ai to back it up.
Summary by CodeRabbit
New Features
Documentation
Tests
Greptile Summary
This PR adds
NotteProvider— a new cloud browser provider for NousResearch/hermes-agent — along with unit tests and a README guide describing how to apply the integration to the external repo. The implementation usesrequeststo drive the Notte REST API session lifecycle (start → debug → stop) with no new dependencies.notte.py):session_data[\"session_id\"]anddebug_data[\"ws\"][\"cdp\"]are accessed without guards; a 200 response with an unexpected JSON shape raises a bareKeyErrorinstead of a clearRuntimeError.TestNotteProviderRegistryimports_PROVIDER_REGISTRYfromtools.browser_tool, which isn't patched in this repo, so that test class will fail as-is.Confidence Score: 4/5
Safe to merge after fixing the unguarded KeyError accesses and addressing the failing registry test.
Two P1 findings remain: unguarded KeyError on API response fields in the session lifecycle, and a test that will always fail in this repo because it depends on a registry change not included here. The implementation itself is well-structured and the remaining findings are P2 style suggestions.
integrations/hermes-agent/tools/browser_providers/notte.py (KeyError risk on response parsing) and integrations/hermes-agent/tests/tools/test_notte_provider.py (registry test always fails).
Vulnerabilities
No security concerns identified. The API key is read from an environment variable and passed as a
Bearertoken header; it is never logged or exposed. No user-controlled input is interpolated into SQL or shell commands.Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: add Notte browser provider integra..." | Re-trigger Greptile