Issue 518 part5 flip default (final part) - #571
Merged
kingpanther13 merged 9 commits intoFeb 15, 2026
Merged
Conversation
…stry tools (homeassistant-ai#518) Part 1 of the isError signaling fix. Adds core infrastructure and applies to tools that already have comprehensive E2E test coverage. Core changes: - Add raise_tool_error() helper to convert error dicts to ToolError exceptions - Update exception_to_structured_error() to raise by default (raise_error=True) - Add @overload type signatures for raise_error parameter - Add safe_call_tool() and tool_error_to_result() test utilities Tool changes (3 tools with existing E2E tests): - tools_config_automations: 5 error paths now raise ToolError - tools_service: 4 error paths now raise ToolError - tools_registry: 1 error path now raises ToolError Test updates: - Update 11 E2E test files to use safe_call_tool for error path testing - Add unit tests for raise_tool_error and exception_to_structured_error - Add retry logic for entity rename tests (timing robustness) Closes part of homeassistant-ai#518 https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8
Per code review feedback: 1. Use match statement for HTTP status code dispatch (Python 3.13) 2. Keep raise_error=False as default to avoid regression during PR stack - Tools explicitly pass raise_error=True or use raise_error=False + raise_tool_error() - Part 5 will flip the default after all tools are migrated 3. Update unit tests to reflect new default behavior https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8
Contributor
Summary of ChangesHello @kingpanther13, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request finalizes the migration to a new error signaling mechanism within the system's tool modules. It primarily involves changing the default behavior of an error handling utility to proactively raise structured 'ToolError' exceptions, ensuring that tool execution failures are consistently communicated at the protocol level. This change, along with the introduction of a dedicated error-raising helper and updates to test infrastructure, streamlines error management and improves the reliability of tool interactions. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
This was referenced Feb 7, 2026
Merged
Merged
Merged
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request successfully completes the migration to using ToolError for signaling errors, which is a significant improvement for error handling consistency and client-side error detection. The changes are applied consistently across all tool modules, and the test suite has been thoroughly updated to reflect this new mechanism, including the addition of robust test helpers like safe_call_tool. The included suggestions for improving context propagation in error messages and enhancing test stability remain valid and are recommended for further improvement.
Fixes for HIGH priority issues: - H1: Add `except ToolError: raise` before `except Exception` handlers in tools_config_automations.py and tools_service.py to prevent double-wrapping - H2: Change return type annotation to `dict[str, Any]` (NoReturn in union is meaningless) - H3: Add case 403 to match statement for auth/permission errors Fixes for MEDIUM priority issues: - M2: Add `default=str` to json.dumps in raise_tool_error for non-serializable fallback - M3: Change pytest.skip to pytest.xfail in test_lights.py to not mask regressions https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8
…omeassistant-ai#518) Part 2 of the isError signaling fix. Updates all remaining E2E test files that expect error responses to use safe_call_tool, which handles both legacy dict returns and new ToolError exceptions. Updated test files: - test_config_entry_flow: 2 error-expecting tests - test_helper_crud: 3 error tests + wait_for_entity_registration helper - test_entity_management: 2 error-expecting tests - test_file_operations: 2 helper functions using parse_mcp_result - test_hacs: 3 error-expecting tests - test_integration_management: 1 error-expecting test - test_device_registry: 4 error-expecting tests - test_voice_assistant: 2 error-expecting tests Total: ~28 error-expecting assertions across 8 files Depends on PR 1 which provides safe_call_tool infrastructure. https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8
…ant-ai#518) Migrate remaining tool modules to use ToolError/raise_tool_error for MCP protocol-level error signaling. Includes updated unit tests for voice assistant and entity tools. https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8
…omeassistant-ai#518) Now that all tools are migrated to use ToolError, flip the default behavior of exception_to_structured_error to raise by default. This completes the ToolError migration - all error paths now signal errors at the MCP protocol level with isError=true. https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8
kingpanther13
force-pushed
the
claude/issue-518-part5-flip-default-U70YN
branch
from
February 7, 2026 19:18
4842dfc to
c9d0493
Compare
Resolve all merge conflicts from upstream/master (Parts 1-4 merged). Address Gemini review comments: - Context propagation now handled by upstream's context=context params (fixes comments #1, #2, #3 on helpers.py) - Replace asyncio.sleep(1.0) with polling loop in test_entity_rename.py (fixes comment #4) Fix callers that assign exception_to_structured_error() result to add explicit raise_error=False (tools_config_dashboards, tools_search) since the default is now True. Fix duplicate ToolError imports in tools_config_helpers and tools_entities. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…_entities Merge artifact from upstream integration — duplicate `from fastmcp.exceptions import ToolError` lines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kingpanther13
marked this pull request as ready for review
February 14, 2026 21:09
kingpanther13
enabled auto-merge (squash)
February 14, 2026 21:10
sergeykad
previously requested changes
Feb 14, 2026
Collaborator
There was a problem hiding this comment.
Consider adding a suggestions parameter to exception_to_structured_error to simplify the ~18 call sites that follow this pattern:
# Current: 3 lines repeated across 18 call sites
error_response = exception_to_structured_error(e, context=ctx, raise_error=False)
error_response["error"]["suggestions"] = ["Check connection", ...]
raise_tool_error(error_response)
# Proposed: 1 line
exception_to_structured_error(e, context=ctx, suggestions=["Check connection", ...])The function embeds the suggestions in the structured error dict before raising. Each of these call sites collapses to one line.
raise_error=False still has one legitimate use: _fetch_state inside ha_get_states needs a classified error dict back (not a raise) because it runs inside asyncio.gather and collects per-entity errors into a bulk response. That case cannot be simplified away.
Address review feedback from @sergeykad — collapse the repeated 3-line pattern (get error dict, insert suggestions, raise) into a single call by adding an optional `suggestions` parameter to `exception_to_structured_error()`. This simplifies ~15 call sites across 7 tool modules while preserving behavior. The `raise_error=False` path remains for `_fetch_state` in ha_get_states which collects per-entity errors inside asyncio.gather. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
@sergeykad Addressed in 6fcea2e — added What changed:
Left unchanged (as noted in your review):
All 610 unit tests pass, ruff check clean. |
sergeykad
approved these changes
Feb 15, 2026
Contributor
🧪 Your changes are now in the dev channel!Your PR has been merged to master and is available for testing in the dev channel. Test your changes before the Tuesday release: Quick start# Run dev version
uvx ha-mcp-dev
# Check version
uvx ha-mcp-dev --versionDocker: docker pull ghcr.io/homeassistant-ai/ha-mcp:dev
docker run --rm -i \
-e HOMEASSISTANT_URL=http://your-ha:8123 \
-e HOMEASSISTANT_TOKEN=your_token \
ghcr.io/homeassistant-ai/ha-mcp:devFound an issue? Please open a new bug report and mention this PR for context. |
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
3 participants
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.
What does this PR do?
Part 5 of 5 for issue #518 — Tool errors not signaled via
isErrorin MCP protocol responses.Flips the
exception_to_structured_error()default fromraise_error=Falsetoraise_error=True, completing the ToolError migration.Changes:
exception_to_structured_error()default toraise_error=True@overloadtype signatures to reflect new defaultWhy this is safe:
All tool modules have been migrated in PRs 1-4 to either:
raise_error=Falsewhen they need to modify the error before raisingraise_tool_error()directlyThis means changing the default has no effect on any existing code paths.
Will completely close #518
Type of change
Testing
uv run pytest)uv run ruff check)Checklist
PR Stack
This is PR 5 of 5 for #518:
tools_mcp_component.py✅raise_error=TrueDepends on: PR 3 #569 must be merged first (all tools must be migrated before flipping the default)