Skip to content

fix: replace bare asserts with explicit exceptions (P0 — python -O survival) - #58837

Open
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/bare-assert-p0-survival
Open

AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/bare-assert-p0-survival

Conversation

@AlexFucuson9

Copy link
Copy Markdown
Contributor

Summary

Replace bare assert statements with explicit exceptions that survive python -O (optimized mode).

Problem

Bare assert statements are stripped silently when Python runs with -O flag. This means critical runtime guards (WebSocket connections, session initialization, container state) simply disappear in production, leading to confusing AttributeError or NoneType crashes with no useful error message.

Changes

File Guard Fix
tools/browser_supervisor.py assert self._ws is not None in _read_loop if ... raise RuntimeError
plugins/google_meet/realtime/openai_client.py assert self._ws in _send_json and _recv if ... raise RuntimeError
gateway/relay/ws_transport.py assert self._ws in _read_loop if ... raise RuntimeError
tools/environments/docker.py assert self._container_id in exec_bash if not ... raise RuntimeError
gateway/platforms/weixin.py 4 session/tensor asserts in poll/send methods if ... raise RuntimeError
tools/skill_manager_tool.py 3 target asserts in write/patch/remove ops if ... return error dict

Impact

  • P0: All 12 fixed asserts were in critical runtime paths (WebSocket I/O, container execution, message polling)
  • All checks already existed but would vanish under -O, creating false confidence
  • Explicit exceptions provide clear error messages and always execute regardless of optimization level

Testing

All 6 files pass python3 -m py_compile verification.

…ival

Bare assert statements are stripped when Python runs with -O flag
(optimized mode), causing silent failures in production. Replace
with explicit exceptions that survive optimization.

Files fixed:
- tools/browser_supervisor.py: WebSocket guard in _read_loop
- plugins/google_meet/realtime/openai_client.py: WebSocket guards
- gateway/relay/ws_transport.py: WebSocket guard in _read_loop
- tools/environments/docker.py: container guard in exec_bash
- gateway/platforms/weixin.py: session guards in poll/send methods
- tools/skill_manager_tool.py: target guards in file operations
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change comp/gateway Gateway runner, session dispatch, delivery backend/docker Docker container execution tool/skills Skills system (list, view, manage) comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 5, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Comment (LGTM)

P0 critical fix: replaces bare assert statements with explicit RuntimeError exceptions. This ensures assertions are not stripped by python -O (optimization flag). The changes span 6 files covering critical initialization and state checks in async/web code paths (WeChat, WebSocket, Google Meet, browser supervisor, Docker).

What looks good:

  • Properly converts validation assertions to actual runtime checks
  • Each replacement includes a descriptive error message
  • Covers critical initialization guards that would cause cryptic failures if stripped

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for addressing a real runtime-safety issue. Current GitHub main still contains the exact bare assertions this PR targets, including tools/browser_supervisor.py:851, gateway/relay/ws_transport.py:607, plugins/google_meet/realtime/openai_client.py:224, and tools/environments/docker.py:1061.

Problems

  • The PR has no regression tests. The diff changes only the six production files, and the current test suite has no coverage for the new error messages or python -O execution. The PR’s stated guarantee should be tested under optimized execution, since normal-mode tests would not catch assertion stripping.

Suggested changes

  • Add focused tests for the uninitialized WebSocket/session/container/skill-target paths, with at least one optimized-interpreter regression path using sys.executable -O.

Automated hermes-sweeper review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/docker Docker container execution comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages tool/skills Skills system (list, view, manage) type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants