refactor: consolidate HACS read tools from 4 to 2 - #871
Merged
kingpanther13 merged 4 commits intoApr 6, 2026
Merged
Conversation
Contributor
Summary of ChangesHello, 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 refactors the HACS toolset to improve maintainability and reduce redundancy. By consolidating the read-only tools, the interface is simplified while retaining all existing functionality. The changes also leverage existing system health checks to provide necessary HACS status information, streamlining the codebase. Highlights
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. Footnotes
|
![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 consolidates HACS tools by removing ha_hacs_info and ha_hacs_list_installed, integrating their functionality into an enhanced ha_hacs_search tool. The updated ha_hacs_search now supports an installed_only parameter, allowing users to list installed repositories or search the full store within a single interface. Corresponding updates were made to the README, documentation, and E2E tests to reflect these changes and ensure continued test coverage using the consolidated tool. I have no feedback to provide.
kingpanther13
marked this pull request as ready for review
April 5, 2026 12:46
kingpanther13
enabled auto-merge (squash)
April 5, 2026 12:46
sergeykad
requested changes
Apr 6, 2026
Merge ha_hacs_list_installed into ha_hacs_search via new installed_only parameter. Remove ha_hacs_info entirely — all its fields are already available via ha_get_system_health. Changes: - ha_hacs_search: add installed_only param (default False), make query optional (default ""). When installed_only=True and no query, lists all installed repos sorted by name. When query is provided, filters by keyword as before. Includes pending_update and domain fields for installed repos. - ha_hacs_info: removed. Version, stage, categories all available via ha_get_system_health. HACS availability already checked internally by _assert_hacs_available() in every HACS tool. - ha_hacs_list_installed: removed. Replaced by ha_hacs_search(installed_only=True). - Updated ha_hacs_repository_info and ha_hacs_download docstrings to reference ha_hacs_search instead of removed tools. - Updated README tool table. - Updated E2E tests: removed TestHacsInfo, converted TestHacsListInstalled to use ha_hacs_search(installed_only=True). Closes homeassistant-ai#833 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Remove dead `or False` after coerce_bool_param (default=False never returns None) 2. Add test for installed_only=True with non-empty query 3. Assert installed_only field in response 4. Convert success:False returns to raise_tool_error in ha_hacs_repository_info, ha_hacs_add_repository, ha_hacs_download (per project guidelines: tool-level failures must raise ToolError) 5. Improve _assert_hacs_available to distinguish "HACS not installed" from "HACS installed but broken" by inspecting error response Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kingpanther13
force-pushed
the
fix/consolidate-hacs-read-tools
branch
from
April 6, 2026 12:10
cd54cf5 to
6814293
Compare
_is_hacs_available was merged into _assert_hacs_available. Tests in test_tools_mcp_component.py that patched the old function now patch _assert_hacs_available instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sergeykad
pushed a commit
that referenced
this pull request
Apr 6, 2026
Fixes 6 of 9 violations caught by the new `no-return-success-false` ast-grep rule (remaining 3 are in tools_hacs.py, addressed by #871). Returning `{"success": False, ...}` from tool functions doesn't set `isError=true` on the MCP response, so LLM agents may not recognize these as errors. Changes: - device_control.py: failed/timeout operation status now raises ToolError - tools_entities.py: ha_set_entity exposure failure now raises ToolError - tools_entities.py: _fetch_entity raises ValueError instead of returning error dict (callers already handle exceptions via return_exceptions) - backup.py: _get_backup_password raises ToolError directly instead of returning (None, error_dict) tuples; callers updated accordingly - test_tools_entities.py: updated test to expect ToolError
This was referenced Apr 6, 2026
Explicitly documents that calling ha_hacs_search() with no query and installed_only=False returns all HACS store repos sorted alphabetically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sergeykad
approved these changes
Apr 6, 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 next stable release (biweekly Wednesday): 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. |
sergeykad
pushed a commit
that referenced
this pull request
Apr 6, 2026
- Revert tools_hacs.py changes (will be handled by #871) - Fix _update_single_entity exposure failure to raise ToolError instead of returning {"success": False} via variable assignment - Re-add exposure_succeeded assertion in test - Update 3 exposure failure tests to expect ToolError
sergeykad
pushed a commit
that referenced
this pull request
Apr 6, 2026
Fixes 6 of 9 violations caught by the new `no-return-success-false` ast-grep rule (remaining 3 are in tools_hacs.py, addressed by #871). Returning `{"success": False, ...}` from tool functions doesn't set `isError=true` on the MCP response, so LLM agents may not recognize these as errors. Changes: - device_control.py: failed/timeout operation status now raises ToolError - tools_entities.py: ha_set_entity exposure failure now raises ToolError - tools_entities.py: _fetch_entity raises ValueError instead of returning error dict (callers already handle exceptions via return_exceptions) - backup.py: _get_backup_password raises ToolError directly instead of returning (None, error_dict) tuples; callers updated accordingly - test_tools_entities.py: updated test to expect ToolError
sergeykad
pushed a commit
that referenced
this pull request
Apr 6, 2026
- Revert tools_hacs.py changes (will be handled by #871) - Fix _update_single_entity exposure failure to raise ToolError instead of returning {"success": False} via variable assignment - Re-add exposure_succeeded assertion in test - Update 3 exposure failure tests to expect ToolError
sergeykad
added a commit
that referenced
this pull request
Apr 6, 2026
* fix: replace hand-built error dicts with raise_tool_error Fixes 6 of 9 violations caught by the new `no-return-success-false` ast-grep rule (remaining 3 are in tools_hacs.py, addressed by #871). Returning `{"success": False, ...}` from tool functions doesn't set `isError=true` on the MCP response, so LLM agents may not recognize these as errors. Changes: - device_control.py: failed/timeout operation status now raises ToolError - tools_entities.py: ha_set_entity exposure failure now raises ToolError - tools_entities.py: _fetch_entity raises ValueError instead of returning error dict (callers already handle exceptions via return_exceptions) - backup.py: _get_backup_password raises ToolError directly instead of returning (None, error_dict) tuples; callers updated accordingly - test_tools_entities.py: updated test to expect ToolError * fix(internal): add ast-grep rule to catch hand-built error dicts The existing `no-return-error-response` rule catches `return create_error_response(...)` but misses hand-built `return {"success": False, ...}` dicts that bypass `raise_tool_error`. These return `isError=false` in MCP responses, so LLM agents may not recognize them as errors. The new rule catches any dictionary with `"success": False` inside a return statement, regardless of key ordering. It correctly excludes batch item appends (`.append(...)`) and dict assignments which are legitimate uses. Currently flags 9 pre-existing violations across 4 files: - tools_hacs.py (3), device_control.py (2), tools_entities.py (2), backup.py (2) * fix: preserve exposure_succeeded in ha_set_entity error context The previous commit dropped partial-success data when converting to raise_tool_error. Restore it via the context dict so LLM agents can see which exposure changes succeeded before the failure. * fix: address review feedback on PR #895 - Revert tools_hacs.py changes (will be handled by #871) - Fix _update_single_entity exposure failure to raise ToolError instead of returning {"success": False} via variable assignment - Re-add exposure_succeeded assertion in test - Update 3 exposure failure tests to expect ToolError
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
2 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?
Consolidates the 4 read-only HACS tools down to 2, based on the discussion in #833:
Merge
ha_hacs_list_installedintoha_hacs_search— addsinstalled_onlyparameter. Both tools already called the samehacs/repositories/listWebSocket endpoint.ha_hacs_search(installed_only=True)replacesha_hacs_list_installed().Remove
ha_hacs_infoentirely — every field it returned is available viaha_get_system_health(version, stage, lovelace_mode). HACS availability is already checked internally by_assert_hacs_available()in every HACS tool. Thecategoriesfield isn't actionable — valid values are already hardcoded asLiteraltypes in the tool parameter schemas.Result: 4 read-only HACS tools → 2 (
ha_hacs_search,ha_hacs_repository_info). The 2 write tools (ha_hacs_add_repository,ha_hacs_download) are unchanged.ha_hacs_search changes
queryis now optional (default"") — empty query withinstalled_only=Truelists all installed reposinstalled_onlyparam (defaultFalse) filters to installed repos onlyinstalled_only=True, response includespending_updateanddomainfields per repoType of change
Testing
uv run pytest)uv run ruff check)Test updates:
TestHacsInfoclassTestHacsListInstalled→TestHacsSearchInstalled, updated to useha_hacs_search(installed_only=True)ha_hacs_infotoha_hacs_searchChecklist
Closes #833
🤖 Generated with Claude Code