fix: replace assert with runtime guards in computer_use (browser_route, doctor) - #80861
Open
JonthanaHanh wants to merge 1 commit into
Open
JonthanaHanh wants to merge 1 commit into
JonthanaHanh wants to merge 1 commit into
Conversation
…e, doctor) assert statements are stripped by python -O, silently removing invariant checks in production. Replace with explicit if/raise guards: - browser_route.py:481 — guard selected_tab and target_id after _require_mutation returns, returning a refusal dict instead of crashing - doctor.py:226 — guard proc.stdin and proc.stdout before JSON-RPC communication, raising RuntimeError with descriptive message
2 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace
assertstatements with explicit runtime guards intools/computer_use/— two sites not covered by prior assert-cleanup PRs (#56736, #56866, #61983, #62001, #62659, #64818).assertis stripped bypython -O, silently removing invariant checks in production.Changes
tools/computer_use/browser_route.pyassert selected_tab is not None and self.state.target_id is not Noneif ... is None: return _refusal(...)tools/computer_use/doctor.pyassert proc.stdin is not None and proc.stdout is not Noneif ... is None: raise RuntimeError(...)Why these matter
browser_route.py: After
_require_mutation()returns with no refusal, subsequent code dereferencesselected_tabandself.state.target_id. Underpython -O, if either is None, the code crashes withAttributeErrordeep in the browser tool dispatch — a confusing error with no context. The guard returns a clear refusal dict.doctor.py:
_mcp_rpc()writes toproc.stdinand reads fromproc.stdout. If either is None (e.g. subprocess started withstdin=DEVNULL), the code crashes withAttributeError: 'NoneType' object has no attribute 'write'. The guard raisesRuntimeErrorwith a descriptive message.Test Plan
python3 -c "import ast; ast.parse(open('tools/computer_use/browser_route.py').read())"— syntax OKpython3 -c "import ast; ast.parse(open('tools/computer_use/doctor.py').read())"— syntax OKruff check