Skip to content

fix(computer-use): add set_value to ComputerUseBackend ABC and _NoopBackend stub - #22748

Closed
rodrigoeqnit wants to merge 1 commit into
NousResearch:mainfrom
rodrigoeqnit:fix/computer-use-noop-set-value
Closed

rodrigoeqnit wants to merge 1 commit into
NousResearch:mainfrom
rodrigoeqnit:fix/computer-use-noop-set-value

Conversation

@rodrigoeqnit

Copy link
Copy Markdown
Contributor

Problem

_dispatch() in tool.py already routes the set_value action to backend.set_value(...), but set_value was never declared as an @abstractmethod on the ComputerUseBackend ABC and was missing from _NoopBackend.

This means:

  • Any concrete backend that forgets to implement set_value passes subclass validation silently.
  • In test mode, calling set_value raises AttributeError on _NoopBackend with no meaningful error message.

Fix

  1. tools/computer_use/backend.py — Added set_value as @abstractmethod to ComputerUseBackend.
  2. tools/computer_use/tool.py — Added a no-op set_value to _NoopBackend that records the call for test inspection.

Tests

Two new test cases in tests/tools/test_computer_use.py:

  • test_set_value_routes_to_backend — verifies dispatch sends correct args.
  • test_set_value_missing_value_returns_error — verifies missing value returns error without raising.

All 46 tests pass.

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets labels May 9, 2026
…ackend stub

_dispatch() routes action="set_value" to backend.set_value(), but:
- ComputerUseBackend did not declare set_value as @AbstractMethod, so
  subclasses could silently omit it without a TypeError at class load time.
- _NoopBackend (the test/CI stub) had no set_value method at all, causing
  AttributeError in any test that exercises the set_value action path.

Fix:
- Add set_value as @AbstractMethod to ComputerUseBackend in backend.py.
- Add a recording stub in _NoopBackend in tool.py.
- Add two TestDispatch cases: one verifying the call reaches the backend,
  one verifying the missing-value guard returns a clean error.
@rodrigoeqnit
rodrigoeqnit force-pushed the fix/computer-use-noop-set-value branch from e63dacb to 4a4b204 Compare May 10, 2026 14:33
@rodrigoeqnit

Copy link
Copy Markdown
Contributor Author

Rebased on current upstream/main (no conflicts). PR is ready for review.

@rodrigoeqnit

Copy link
Copy Markdown
Contributor Author

Hi maintainers — this PR has been open for 9 days without feedback. It makes set_value an abstractmethod in the computer-use backend and adds a NoopBackend stub so subclasses are forced to implement it properly. Happy to adjust if needed. Thanks!

@teknium1

Copy link
Copy Markdown
Collaborator

Salvaged via PR #30270 (commit 07b7cf6 on main). Your commit was cherry-picked onto current main with your authorship preserved. Thanks for the fix!

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

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants