feat: rename ha_delete_helpers_integrations → ha_remove_helpers_integrations + raise on missing target

[Patch76]

What does this PR do?

Closes #1412.

Renames ha_delete_helpers_integrations → ha_remove_helpers_integrations and unifies the tool's behavioural contract across all four routing paths under a single missing-target contract that raises a structured error on confirmed-absent targets.

Rename rationale

The _remove_* naming family (12 tools) all carry idempotentHint: True and operate on "make the target not-there" semantics. The previous _delete_ prefix put this tool with two other _delete_ tools that have three different not-found behaviours (no convention). Joining the _remove_* family + adding idempotentHint makes the contract explicit.

Per .gemini/styleguide.md § Breaking Changes, "Tool renaming with clear migration path" is explicitly listed under NOT Breaking (alongside tool consolidation, refactoring, parameter changes, and return-value restructuring). The migration path here is the conventional-commit rename in this PR.

Missing-target contract

A target that is confirmed absent raises a structured error rather than returning silent success — the typo / hallucinated-identifier case surfaces immediately at the caller layer instead of being masked as a false success. Error codes follow the target shape per path:

Path	helper_type	Confirmed-absent path	Error code	Other failure modes
1 SIMPLE	input_button, input_boolean, …	state + registry both empty (incl. never-existed targets where get_entity_state 404s)	ENTITY_NOT_FOUND	unique_id missing but registry still has entry → SERVICE_CALL_FAILED
2 FLOW	utility_meter, template, …	step-1 not_in_registry	ENTITY_NOT_FOUND	step-3 TOCTOU 404 on resolved entry_id → RESOURCE_NOT_FOUND; no_config_entry (YAML) → RESOURCE_NOT_FOUND; lookup_failed (transient) → WEBSOCKET_DISCONNECTED; bare_id_not_supported → ENTITY_NOT_FOUND
3 Direct entry	None	HA REST returns 404 (HomeAssistantAPIError.status_code == 404)	RESOURCE_NOT_FOUND	non-404 API errors → exception_to_structured_error
4 Subentry	config_subentry	HA WS structured error code: "not_found" only (generic message substring match dropped per Gemini review to avoid colliding with unrelated "X not found" errors)	RESOURCE_NOT_FOUND	other WS failure modes → SERVICE_CALL_FAILED

ENTITY_NOT_FOUND for entity-shaped targets (Path 1, Path 2 step-1) matches the existing bare_id_not_supported branch within Path 2 and sibling ha_remove_entity. RESOURCE_NOT_FOUND for config-shaped targets (Path 2 step-3, Path 3, Path 4) matches Path 2's existing no_config_entry branch and sibling ha_config_remove_automation.

Idempotency at the contract level still holds: N calls produce the same response. The earlier draft of this PR returned silent success on absent — that conflated idempotent ("no additional effect on repeat") with tolerant ("absent → success"); only the first is part of the contract. Transient connectivity errors keep their separate codes (WEBSOCKET_DISCONNECTED, CONNECTION_ERROR) so retry logic can branch on those independently.

Surface touched

• src/ha_mcp/tools/tools_integrations.py: rename + idempotentHint: True annotation + four per-path raise branches on confirmed-absent + docstring with full MISSING-TARGET contract.
• src/ha_mcp/tools/tools_groups.py: 3 cross-reference docstrings.
• ~130 mechanical rename occurrences across 21 test files + 1 cleanup utility + README.
• 6 unit tests flipped from assert fallback_used="already_deleted" to pytest.raises(ToolError) with per-path code (5 in test_tools_integrations.py, 1 in test_config_subentries_folded.py); the pre-existing negative-assertion tests ("already_deleted" not in json.dumps(err)) stay green and continue to pin the no-mislabeling rule across 404 and non-404 paths.
• 2 E2E tests flipped to assert the structured failure dict.
• site/src/data/tools.json + README.md + homeassistant-addon/DOCS.md regenerated via scripts/extract_tools.py.
• CHANGELOG.md historical entries referencing the old name intentionally untouched.

Type of change

• 🐛 Bug fix
• ✨ New feature
• 📚 Documentation
• 🔧 Maintenance/refactor
• 🧪 Tests only
• 💥 Breaking change

Testing

• I have tested these changes with a LLM agent
• All automated tests pass (uv run pytest)
• Code follows style guidelines (uv run ruff check)

Local verification:

• ruff check src/ tests/ — All checks passed
• ruff format on changed files
• mypy src/ha_mcp/tools/tools_integrations.py — no issues
• 76 unit tests in test_tools_integrations.py + test_config_subentries_folded.py pass after rework

Checklist

• PR title follows the convention used in repo (semver-release Angular parser)
• Commits follow conventional commits format
• Code is properly tested (or tests-not-needed label is added)
• Documentation is updated if needed
• No unrelated changes are included

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 helper deletion tool by renaming it to ha_remove_helpers_integrations and establishing a robust idempotent post-state contract. By ensuring that requests to remove non-existent resources return a success status, the tool becomes more reliable for automated agents. The changes include comprehensive updates to documentation, test suites, and internal tool routing logic.

Highlights

• Tool Rename: Renamed the tool ha_delete_helpers_integrations to ha_remove_helpers_integrations to align with naming conventions.
• Idempotent Contract: Implemented a unified idempotent post-state contract across all four routing paths, ensuring that attempts to remove already-absent targets return a success response.
• Tool Annotation: Added idempotentHint: True to the tool definition to make the new contract explicit.
• Documentation: Updated README, DOCS.md, and tool metadata to reflect the rename and new behavior.
• Testing: Updated numerous unit and E2E tests to use the new tool name and verify the idempotent behavior.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request renames the ha_delete_helpers_integrations tool to ha_remove_helpers_integrations and implements an idempotent-success contract across all deletion paths, ensuring that requests to remove already absent targets return a successful response instead of an error. Feedback was provided to refine the error message string matching for subentry deletions by using more specific substrings to prevent false positives, in accordance with repository guidelines.

[Patch76]

/gemini review

[gemini-code-assist]

Code Review

This pull request renames the ha_delete_helpers_integrations tool to ha_remove_helpers_integrations and implements an idempotent-success contract across its various routing paths, ensuring that targets already absent from Home Assistant return a successful response. Feedback focuses on refining exception handling to align with repository standards; specifically, several broad except Exception blocks need to be narrowed to prevent masking programming errors, and an API error catch in the simple helper path should be restricted to 404 status codes to correctly identify absent entities.

[Patch76]

/gemini review

[gemini-code-assist]

Code Review

This pull request renames the ha_delete_helpers_integrations tool to ha_remove_helpers_integrations and implements a comprehensive idempotent-success contract across all deletion paths (Simple, Flow, Direct, and Subentry). The changes ensure that targets already absent from Home Assistant return a success state rather than an error. Feedback focuses on maintaining consistency by removing redundant logging when delegating to centralized error handlers and ensuring the response schema for config subentry deletion matches the other logic branches by including missing keys like entry_id and entity_ids.

[Patch76]

/gemini review

[gemini-code-assist]

Code Review

This pull request renames the ha_delete_helpers_integrations tool to ha_remove_helpers_integrations and implements an idempotent-success contract across all deletion paths. The tool now returns a successful response with a specific fallback indicator when a target is already absent, specifically handling 404 errors from the Home Assistant API and missing registry entries. Safety annotations were updated to include idempotentHint, and comprehensive updates were applied to documentation and tests to reflect these changes. I have no feedback to provide.

[Patch76]

Implementation Summary

Choices Made:

• Original choice: Path (c) from issue [FEATURE] decide ha_delete_helpers_integrations idempotent dual-success contract #1412 — rename ha_delete_helpers_integrations → ha_remove_helpers_integrations, add idempotentHint: True, and unify all four routing paths under an idempotent-success contract that returned success=True, fallback_used="already_deleted" on confirmed-absent.
• Reworked under review pressure to Path (b) — kp13's review identified that success=True on a confirmed-absent target is a silent false-success for wrappers that branch on success: a typo'd or hallucinated identifier would surface as a successful removal while the real target remained. The original framing also conflated idempotent ("N calls = same result") with tolerant ("absent → success"). All five already_deleted branches were flipped to raise_tool_error with structured codes; idempotency at the contract level still holds (N calls = same raise).
• Per-target-shape error codes rather than uniform RESOURCE_NOT_FOUND. Empirically Path 2 already differentiated internally (bare_id_not_supported → ENTITY_NOT_FOUND, no_config_entry → RESOURCE_NOT_FOUND), so the new branches were classified the same way:
  • Path 1 SIMPLE state-gone + registry-empty (entity-shape) → ENTITY_NOT_FOUND. Matches sibling ha_remove_entity.
  • Path 2 FLOW not_in_registry (entity-shape) → ENTITY_NOT_FOUND. Matches Path 2's existing bare_id_not_supported branch.
  • Path 2 FLOW TOCTOU 404 (resource-shape, post-resolution) → RESOURCE_NOT_FOUND with a distinct "concurrent removal" diagnostic hint (the entry WAS resolvable at step 1).
  • Path 3 Direct REST 404 (config_entry shape) → RESOURCE_NOT_FOUND.
  • Path 4 Subentry code: "not_found" (subentry shape under parent entry) → RESOURCE_NOT_FOUND.
• Diagnostic-hint wording on each raise carries the structured "may indicate already removed, never existed, or typo. Verify with ha_search_entities() / ha_get_integration() before retrying" framing so the caller-layer (isError per MCP spec) gets the typo signal regardless of whether the wrapper surfaces error messages. Pinned by substring assertions in the raise-branch tests so future edits can't silently degrade the wording.
• Deliberately kept (Boy-Scout-decided as out-of-scope-for-rename):
  • Internal helper method names _delete_simple_helper, _delete_flow_helper, _delete_direct_entry, _delete_config_subentry — these describe the underlying HA backend operation (HA's API uses delete), not the public tool verb.
  • "action": "delete" field in the response shape — backward compatibility for any LLM agent or downstream tool that branches on the action label.
• Broad except Exception blocks left as-is (Gemini findings on the broad-except idiom in this module): the except ToolError: raise + except Exception → exception_to_structured_error(e, ...) pair is the idiom for outer handlers in this module — six places use it. Narrowing only the two flagged sites would create inconsistency; a repo-wide narrowing sweep is the appropriate vehicle.

Problems Encountered:

• E2E test_helper_delete_nonexistent failed on the first push with ENTITY_NOT_FOUND instead of the documented already_deleted (pre-rework state). Root cause: Path 1's existing already_deleted short-circuit only fired when get_entity_state returned None, never when it raised HomeAssistantAPIError(status_code=404) — and HA raises 404 for never-existed entity_ids. Fix in 49e3cdd: extracted state_gone boolean to handle both falsy return AND 404 APIError. (Post-rework this fix still routes the 404 case into the confirmed-absent classification, but the result is now a structured raise rather than success.)
• First Path 1 APIError catch was too permissive — would have classified ANY HomeAssistantAPIError (500, 401, …) as state-gone. Caught on review; narrowed to if e.status_code != 404: raise in e9e13ff. Mirrors the e.status_code == 404 guard in _delete_direct_entry.
• Path 4 substring match "not found" in error_msg.lower() would have classified unrelated HA errors ("Repository not found", "Integration not found") as the confirmed-absent path. Tightened to structured code: "not_found" only in bf50016. The HA contract was verified live against home-assistant/core source: config_entries/subentries/delete raises with ERR_NOT_FOUND: Final = "not_found".
• kp13's review-1 (R23) called the whole idempotent-success contract a silent false-success. Live testing against the deployed RBO instance (Phase A) confirmed empirically: master ha_delete_helpers_integrations raises on typo targets across Paths 1/2/3, sibling ha_remove_entity raises ENTITY_NOT_FOUND, ha_config_remove_automation raises RESOURCE_NOT_FOUND. The PR's pre-rework contract was the outlier, not the consolidation. Reworked in 20799d7 (impl) + 48a4f7b (tests).
• Empirically the sibling family is not uniform on the not-found code (a side discovery of Phase A): ha_remove_zone and ha_remove_area_or_floor surface SERVICE_CALL_FAILED rather than *_NOT_FOUND. Out of scope for this PR — flagged for separate code-cleanup.
• Ultrareview pass on the post-rework state surfaced two docstring NITs: CONNECTION_ERROR was a non-existent code name (real one is CONNECTION_FAILED per errors.py:33), and the wait parameter's "Ignored when helper_type=None" note didn't name config_subentry (Path 4 also drops wait). Both fixed + site/src/data/tools.json regenerated in 7449c43.
• Post-rework ruff format drift on test_tools_integrations.py — ruff format collapsed two multi-line function signatures back to single-line under the 88-char width. Caught by CI Ruff Lint on 7449c43; fixed in 98f6ed2.
• kp13's review-2 surfaced two test-coverage gaps: a missing "already_deleted" not in json.dumps(err) regression pin on test_remove_helpers_integrations_subentry_other_error_surfaces_service_call_failed, and an unpinned diagnostic-hint wording on the six raise-branch tests. Both addressed in bd48061 — the diagnostic hint substring is now part of the test assertion surface on every confirmed-absent path.
• ha_backup_create reporting drift discovered during Phase A live testing (not in scope for this PR): the tool returns success: false, code: TIMEOUT_OPERATION even when the backup actually succeeds on disk. Worth filing as a separate issue.

Suggested Improvements:

None deferred. Every NIT / Nice-to-have / While-you're-in flagged across the review cycles (Gemini Findings 1-8, kp13 reviews 1+2, ultrareview pass) was addressed in-PR per feedback_nits_are_mandatory + Boy Scout Rule. The broad-except Exception pattern flagged by Gemini Findings 1+2 is the only item not in-PR — narrowing only the two flagged sites would create inconsistency with the four other places in this module using the same idiom; the right vehicle is a file-wide or repo-wide sweep.

[kingpanther13]

Walking through Path 1 (SIMPLE) for a typo'd never-existed target — e.g. agent calls target="my_buton", helper_type="input_button" when only my_button exists:

On upstream/master (_delete_simple_helper, lines 1685–1787): registry retries 3× → no unique_id → direct-ID fallback delete fails → final get_entity_state("input_button.my_buton") raises HomeAssistantAPIError(404) (HA returns 404 for never-existed entity_ids) → caught at line 1760, logged as debug, falls through to line 1773 → raises ENTITY_NOT_FOUND.

On this PR (same method, lines 1819–1878): the 404 from that final state check now sets state_gone=True, registry verify confirms no entity, and the tool returns success=True, fallback_used="already_deleted". This is the change called out in commit 49e3cdd7: "Path 1's existing already_deleted short-circuit only fired when get_entity_state returned None, never when it raised HomeAssistantAPIError(status_code=404) — and HA raises 404 for never-existed entity_ids."

So for the typo / hallucinated-target case — agent meant my_button, called with my_buton — this PR changes behavior from "raises `ENTITY_NOT_FOUND`" to "returns `success=True`". An agent wrapper branching on `success` would report deletion succeeded while the real `my_button` is untouched. (Paths 2/3/4 newly adopt the same shape, but Path 1 is the one where existing callers see different behavior on never-existed inputs.)

Three things to make sure I'm reading the intent correctly:

1. Is the typo / hallucinated-target case in-scope for the `confirmed absent → success` contract? My read of the PR text and the `49e3cdd7` fix is yes (never-existed and already-deleted both count as confirmed absent), but I want to verify that's intentional and not just a side-effect of fixing the e2e test failure.

2. Is `fallback_used == "already_deleted"` the intended signal for callers to distinguish "actually removed" from "was never there"? Do any of the e2e tests or known agent harnesses currently branch on that field, or is that left to the caller?

3. Was the prior Path 1 behavior (raise `ENTITY_NOT_FOUND` on never-existed) viewed as a bug to fix here, or as a contract being deliberately replaced? The PR notes read as the former; want to confirm I'm not misreading.

[kingpanther13]

The clarifying comment above raises one design question I want resolved before merge. The rest of this review is the substantive findings from a full pass over the diff — please treat as a single bundle, not a checklist of optional nits.

Blocking — contract correctness

1. Path 4 happy-path response shape diverges from the uniform contract.
_delete_config_subentry at src/ha_mcp/tools/tools_integrations.py:1675-1684 omits entry_id, entity_ids, and require_restart on the success path. The Path 4 idempotent branch (1652-1667) includes them. The IDEMPOTENT-SUCCESS CONTRACT docstring (1223-1232) implies a uniform response shape across paths; this break is the most likely place a caller's downstream parsing chokes silently. Either add the three keys to the happy path or remove them from the idempotent branch and document the asymmetry.

2. Path 4 happy-path leaks a result passthrough key no other path returns (tools_integrations.py:1683). Drop it or hoist into the contract docstring as a Path-4-only addition.

3. Path 4 only matches error.code == "not_found" when error is a dict (tools_integrations.py:1650-1651). The narrow is correct, but there is no test that pins it — a regression that loosened to substring-matching "not found" on string-form errors would pass CI silently. Add a negative test asserting that {"success": False, "error": "not found"} raises SERVICE_CALL_FAILED and does NOT classify as idempotent.

4. Path 3 has no negative test for non-404 HomeAssistantAPIError on delete_config_entry. Paths 1, 2, and 4 each have a paired non-target-shape negative pinning the narrow; Path 3 doesn't. Add one (e.g. 500 or 401 from the delete call must surface as a structured error, not idempotent success).

5. Two test assertions are tautological in their failure mode and don't actually pin the intent.

• tests/src/unit/test_tools_integrations.py:391-393: assert err["error"]["code"] != "ENTITY_NOT_FOUND" or "already_deleted" not in ... passes whenever the left disjunct is true, regardless of the right; a regression that leaks fallback_used into a non-ENTITY_NOT_FOUND error wouldn't fail this test. Replace with a single positive assertion on the expected code AND a separate assert "already_deleted" not in str(err).
• tests/src/unit/test_tools_integrations.py:440-441: HomeAssistantAPIError("404 simulated") is constructed without status_code, so the test never exercises the status_code == 404 branch it claims to cover. Pass status_code=404 explicitly.

Blocking — docstring / contract accuracy

6. The IDEMPOTENT-SUCCESS CONTRACT docstring (tools_integrations.py:1223-1232) does not acknowledge the typo/never-existed equivalence on Paths 1 and 2. This is the same concern raised in the clarifying comment. Whatever the resolution there, the docstring needs to say plainly that a never-existed target (e.g. a typo'd target) is indistinguishable from an already-removed target and returns the same idempotent success — so callers reading the contract aren't surprised.

7. Verb drift across target, helper_type, subentry_id, confirm, wait Field descriptions (tools_integrations.py:1117-1188) — all still say "delete"/"deletion" while the tool verb and title are "Remove". Mixed verbs in the public schema. Fix in this PR.

8. Stale path-count comments: tools_integrations.py:1264 and 1282 both say "uniform for all three paths" after Path 4 (config_subentry) was added. Either four or three — pick one and match.

9. Stale tool-name reference in docs/superpowers/specs/2026-05-21-1288-auto-backup-design.md:234 — still says ha_delete_helpers_integrations. Rename to ha_remove_helpers_integrations to match the auto-backup decorator on the renamed method.

Blocking — error-handling

10. _get_entry_id_for_flow_helper broad except Exception (tools_integrations.py:144-148) maps KeyError/AttributeError/TypeError (i.e. programmer bugs) to lookup_failed, which the caller then converts to WEBSOCKET_DISCONNECTED — mis-labeling local bugs as transient network errors. Narrow to the exception types you actually expect from the WS client.

While you're in there (small)

• One-line module comment somewhere near the private helpers explaining that internal _delete_* names follow the HA backend verb (HA's WS API is <type>/delete, REST is DELETE), while the public tool exposes remove. Prevents future rename drift pressure.
• The "Combines simple-helper websocket deletion, config-entry deletion, and config-subentry deletion under one entry point with four routing paths" line (1194-1196) is "three deletion kinds in four paths" — phrasing reads as a typo. Tighten.
• The FLOW contract bullet (1228-1230) lists no_config_entry raising RESOURCE_NOT_FOUND but doesn't mention bare_id_not_supported (raises ENTITY_NOT_FOUND); per-path contract reads as exhaustive while it isn't. Add the third FLOW failure mode or note the list isn't exhaustive.
• The state_gone block comment at tools_integrations.py:1813-1818 restates what the next 6 lines literally do; a one-liner pointing at the never-existed-target consequence carries the load.

[Patch76]

Re: clarifying comment on Path 1 typo / hallucinated-target behavior

Intentional on all three questions. Walking through:

1. Typo / never-existed case IS in-scope for the "confirmed absent → success" contract. Issue [FEATURE] decide ha_delete_helpers_integrations idempotent dual-success contract #1412 framing is agent-MCP idempotency, and at the HA backend layer "never existed" and "was deleted" are indistinguishable — both surface as HomeAssistantAPIError(status_code=404) on get_entity_state. The contract is "confirmed absent from HA's perspective"; HA cannot validate the target was ever real.

2. fallback_used == "already_deleted" IS the explicit caller-signal for "this call did not change state". Agents needing to distinguish "actually removed by this call" vs "was already gone" must check externally — state history, audit log, or a pre-flight get_entity lookup. The tool does not disambiguate; that's by design.

3. Path 1's prior ENTITY_NOT_FOUND-on-never-existed was a path-internal inconsistency, not deliberate contract. Path 1 already had an already_deleted short-circuit that fired when get_entity_state returned None, but the 404-raising branch — the path HA actually takes for never-existed entity_ids — fell through to the ENTITY_NOT_FOUND raise. Commit 49e3cdd7 makes Path 1 self-consistent; same shape Paths 2/3/4 now adopt.

The agent-wrapper concern (success-branch reports deletion when caller meant a different existing target) is real. Mitigation lives in your item 6 — the IDEMPOTENT-SUCCESS CONTRACT docstring will say plainly that "confirmed absent" includes the never-existed case and that callers needing disambiguation must do so externally. Folding into the next round, alongside the 13 other items.

[Patch76]

Review-pass status

Pushed f3d9ac3: 8 of 10 numbered items + all 4 while-you're-in NITs addressed. CI re-running. The remaining 2 items are intentionally held pending your design-clarification answer.

Addressed in this push

Item	Change
2	Drop result passthrough on Path 4 happy-path (no other path returns it)
3	New negative test pins Path 4 dict-only narrow against string-form "error": "not found" regressions
4	New negative test pins Path 3 narrow against non-404 HomeAssistantAPIError (500 / 401)
5a	Tautological OR-disjunct → two independent assertions: code != "ENTITY_NOT_FOUND" AND "already_deleted" not in json.dumps(err)
5b	HomeAssistantAPIError("404 simulated") now passes status_code=404 — the 404 narrow it claims to cover is actually exercised
7	Verb drift delete / deletion → remove / removal in the target, helper_type, subentry_id, confirm Field descriptions
8	uniform for all three paths → four (both confirm and empty-target gates); Path 4 added to empty-target enumeration
9	Stale ha_delete_helpers_integrations reference in docs/superpowers/specs/2026-05-21-1288-auto-backup-design.md updated
10	_get_entry_id_for_flow_helper broad except Exception → (OSError, TimeoutError). KeyError / AttributeError / TypeError now propagate as INTERNAL_ERROR instead of being mis-classified as WEBSOCKET_DISCONNECTED
NIT-a	Module comment near the private _delete_* helpers explaining the HA-backend-verb vs public-remove-family prefix asymmetry
NIT-b	Combines simple-helper websocket deletion, config-entry deletion, and config-subentry deletion under one entry point with four routing paths → Unifies three backend removal mechanisms — simple-helper websocket delete, config-entry delete, and config-subentry delete — behind one entry point with four routing paths
NIT-c	FLOW contract bullet now lists bare_id_not_supported as a third failure mode (raises ENTITY_NOT_FOUND)
NIT-d	state_gone block comment compressed — one note on the never-existed-target consequence of the APIError-404 branch

Held — pending your design call

Two items intentionally deferred. Both depend on your read of the design-clarification answer:

• Item 1 — Path 4 happy-path schema harmonization (add entry_id / entity_ids / require_restart to the success-path return). Direction depends on whether the intended contract is "uniform schema across all four idempotent and happy paths" (add) or "asymmetric and documented" (drop from idempotent + document). Currently leaning uniform.
• Item 6 — IDEMPOTENT-SUCCESS CONTRACT docstring rewrite acknowledging the typo / never-existed equivalence on Paths 1 + 2. Phrasing depends on whether you ack the typo-case-in-scope read.

Both land in a follow-up commit on the same review round once your direction is clear.

[kingpanther13]

On item 6's docstring mitigation: a docstring caveat won't carry the load. Agent wrappers branch on the success boolean — that's the runtime signal that shapes behavior loops. Docstrings get read once at tool-registration time (often not at all in long sessions); they don't gate per-call decisions. An LLM that sees success: True will report "done" and move on regardless of what the docstring said. And pushing the disambiguation onto callers ("pre-flight get_entity lookup, audit log, state history" — your reply #2) shifts cost per-call onto every consumer; the failure mode when they forget lands on end users who get told a delete succeeded when their real target is still there.

A third option that doesn't break your idempotent contract: keep success=True, populate the project's existing top-level warnings: list[str] field on the already_deleted branches:

warnings=[
    "Target was already absent. If this was unexpected, "
    "verify the identifier — may indicate a typo or stale reference."
]

AGENTS.md § Return Values + tools_config_helpers.py::HelperResponse already standardize the shape; tests/src/unit/test_helper_response_shape.py enforces it. The project pattern is that agents surface warnings to users — most tools that produce them do. The typo signal fires at runtime where it actually catches things, docstring stops being load-bearing for safety, and your idempotent semantic stays intact.

Separately, I'm researching whether there's a better alternative than warnings for the underlying disambiguation. HA does retain a deleted_entities tombstone internally (homeassistant/helpers/entity_registry.py:1116 — dict[(domain, platform, unique_id), DeletedRegistryEntry]), but no public WS or REST endpoint exposes it today — I checked the registered commands in homeassistant/components/config/entity_registry.py:26-32. If a future surface (ours or upstream's) makes that data queryable, the disambiguation could move out of warnings and into a real signal. For now warnings is the realistic option.

If it doesn't work for you, I'm open to other shapes — but I don't want to land item 6 as "docstring says read fallback_used." That just leaks typos into user-facing reports.

[kingpanther13]

Stupid Claude... Issue isn't "leaks typos into user reports." The issue is that it will return false successes to users, which is a problem... If it tries to delete the wrong thing, then Things that users thought got deleted will still exist, and won't have a good way to know that that even happened. Big silent failure/false success.

[kingpanther13]

Following up on the warnings proposal — pulling back from that. Re-reading, putting the safety signal in warnings has the same shape of problem as the docstring, one rung up: it's optional for the wrapper to surface. The cohort that doesn't surface warnings is the same cohort that branches naively on success — exactly the wrapper class where end users get told a delete succeeded when their real target is still there.

Leaning toward Option B. Concrete shape on the already_deleted branches:

raise_tool_error(create_error_response(
    ErrorCode.RESOURCE_NOT_FOUND,
    f"Target {target} not found. This may indicate it was already removed, "
    "never existed, or the identifier is a typo. Verify with ha_search_entities() "
    "or ha_get_integration() before retrying.",
    context={"target": target, "helper_type": helper_type},
))

What this gets:

• Aligns with the 12 sibling _remove_* tools — uniform "raise RESOURCE_NOT_FOUND on missing" contract across the family
• Typo signal preserved at the layer every wrapper reads (isError per MCP spec)
• Repeated calls return the same response — still idempotent at the contract level (call N times = same result). The "absent → success" framing was conflating idempotent with tolerant; those are different semantics, and only the first one the contract needs

Your argument for "absent → success" was retry-on-network-drop. But transient HA errors are already separately classified (WEBSOCKET_DISCONNECTED, CONNECTION_ERROR, …) — agent retry logic branches on those codes, not on every error. RESOURCE_NOT_FOUND triggers correction or escalation in well-built wrappers, and a tolerable extra round-trip in legitimately-retrying ones. The typo case is where the asymmetry actually bites.

On the underlying data — HA does retain deleted_entities internally (homeassistant/helpers/entity_registry.py:1116). Claude hasn't surfaced a good way to access it yet — no public WS endpoint like config/entity_registry/list_deleted or get_deleted_by_id, and no REST equivalent that I've found so far. Still researching; will report back if anything turns up.

[kingpanther13]

Closing the research loop on disambiguating typo vs already-deleted. The tombstone exists on disk in .storage/core.entity_registry and in HA process memory as registry.deleted_entities, but no public API exposes it. I exhausted:

• All config/entity_registry/* WS commands — return only registry.entities (verified by raw WS dump against my live 5614-entry registry: the string "deleted appears 0 times in the 3.92 MB response)
• area_registry / device_registry cross-references — no hits
• All speculative names (list_deleted, get_by_id, dump, list_with_deleted, …) — all return unknown_command from HA's WS router
• Jinja templates — read all 21 files in homeassistant/helpers/template/extensions/, zero deleted references; verified live (entity_name("input_button.fake") returns None)
• Recorder/history — retains state changes within recorder.purge_keep_days (default 10), but rolls off after retention → not contract-grade

Spot-checked a known deleted entity in my install (input_boolean.bat_1152_loop_0, registry id eb4ae8af54a68465cf08ba8949164d4a) — not retrievable via any active-registry endpoint despite being in the on-disk tombstone array.

This all points me to Option B. Go ahead and rework items 1 and 6 to raise RESOURCE_NOT_FOUND on confirmed-absent — concrete shape in my prior comment. I'm done researching the disambiguation path.

[Patch76]

Reworked items 1 and 6 to raise on confirmed-absent across all five already_deleted branches. Three commits on top of f3d9ac3:

• fix(tools_integrations): raise on confirmed-absent target instead of silent success (20799d7) — flips the five branches to raise_tool_error, rewrites the docstring contract block as MISSING-TARGET CONTRACT.
• test(tools_integrations): flip absent-target tests to raise assertions (48a4f7b) — flips 6 unit tests (5 in test_tools_integrations.py, 1 in test_config_subentries_folded.py) and 2 E2E tests; the pre-existing negative "already_deleted" not in json.dumps(err) assertions stay and continue to pin no-mislabeling.
• fix(tools_integrations): address ultrareview NITs on rework docstring (7449c43) — boy-scout pass on two docstring NITs surfaced by a /ultrareview run on the post-rework state: CONNECTION_ERROR was a wrong code name (real one is CONNECTION_FAILED per errors.py:33), and the wait param's "Ignored when helper_type=None" note was extended to also name config_subentry (Path 4 also drops wait). Regenerated site/src/data/tools.json via scripts/extract_tools.py so the catalog matches the live docstring.

PR body updated with the new contract table.

Branches flipped

#	Path	Branch	Now raises
1	Path 3 _delete_direct_entry	REST 404 on entry_id	RESOURCE_NOT_FOUND
2	Path 2 _delete_flow_helper step 1	not_in_registry	ENTITY_NOT_FOUND
3	Path 2 step 3	TOCTOU 404 on resolved entry_id	RESOURCE_NOT_FOUND
4	Path 4 _delete_config_subentry	WS error.code == "not_found"	RESOURCE_NOT_FOUND
5	Path 1 _delete_simple_helper	state-gone + registry-verify empty	ENTITY_NOT_FOUND

Note on the error code split

Your concrete code snippet used uniform RESOURCE_NOT_FOUND. I went with per-target-shape codes (ENTITY_NOT_FOUND for entity-shaped targets in branches 2 + 5, RESOURCE_NOT_FOUND for config-shaped in branches 1 + 3 + 4) because Path 2 already differentiates internally: the existing bare_id_not_supported branch at line 1514 (pre-rework) raises ENTITY_NOT_FOUND, and the no_config_entry branch at line 1447 raises RESOURCE_NOT_FOUND. Flipping branch #2 to RESOURCE_NOT_FOUND would have left Path 2 with two codes for absent-target ("bare_id" → ENTITY_NOT_FOUND, "not_in_registry" → RESOURCE_NOT_FOUND), which felt like an internal inconsistency worth avoiding. Branch #5's ENTITY_NOT_FOUND matches sibling ha_remove_entity.

The message text per branch keeps the "may indicate already removed, never existed, or typo. Verify with ha_search_entities() / ha_get_integration() before retrying." shape from your snippet — wrappers branching on isError get the structured signal regardless of which code variant landed.

Happy to flip to uniform RESOURCE_NOT_FOUND if you prefer it, including the existing bare_id_not_supported branch for full internal consistency — let me know.

Verification

Local: 76 pytest items in test_tools_integrations.py + test_config_subentries_folded.py pass after the rework (71 unique test functions, expanded to 76 via one parametrize over 6 helper types; 6 of the 71 functions are reworked, the rest unchanged). CI pipeline kicks off on this push — the 2 flipped E2E tests are the post-deploy verification that the live tool surfaces the new codes against an actual HA instance.

On the disambiguation research closure (your comment from 13:11Z): agreed, no usable signal exists in the active-registry surface today. The deleted_entities tombstone array would have been the right backbone if it had a WS query, but it doesn't. The raise-on-confirmed-absent contract is the realistic shape.

[kingpanther13]

Rework lands cleanly: five branches raise as requested, docstring contract matches code, error messages include verification hints. On your design question — keep per-target-shape (ENTITY_NOT_FOUND for entity targets, RESOURCE_NOT_FOUND for config targets). It matches the existing bare_id_not_supported (entity-shaped) vs no_config_entry (config-shaped) split in Path 2 and aligns Path 1 with sibling ha_remove_entity. Uniform RESOURCE_NOT_FOUND would have created an internal inconsistency.

Two items before this lands:

1. Ruff format failing in CI on tests/src/unit/test_tools_integrations.py. Per the Ruff Lint job log:

Would reformat: tests/src/unit/test_tools_integrations.py
1 file would be reformatted, 23 files already formatted

Run ruff format tests/src/unit/test_tools_integrations.py and push.

2. Missing "already_deleted" not in json.dumps(err) regression pin on test_remove_helpers_integrations_subentry_other_error_surfaces_service_call_failed (tests/src/unit/test_config_subentries_folded.py:535). Its sibling string-form-error test at line 589 has the pin; this one doesn't. A regression that mis-routes a non-not_found error into the already_deleted shape wouldn't fail this test. Add the assertion.

While you're in there

None of the five raise-branch tests assert the diagnostic hint content in the error message beyond the target id — a regression that strips the "may indicate it was already removed, never existed, or the identifier is a typo. Verify with ha_search_entities()/ha_get_integration()" wording wouldn't fail any test. A "may indicate" in err["error"]["message"] (or equivalent substring check) on each of the five branches would pin the user-facing diagnostic so the message can't silently degrade — we just spent a review cycle landing that exact wording.

HAOS E2E failure noted as flaky — not requesting any change there.

[Patch76]

Thanks for the design ack on the per-target-shape split.

Addressed both blockers + the "while you're in there" in a single commit:

Item 1 — Ruff format on test_tools_integrations.py

Already addressed in 98f6ed2 — pushed at 16:13 UTC, ~7 min before your review submission at 16:20 UTC. The review was filed against the prior commit 7449c43, so the Ruff CI fix wasn't reflected in the state you reviewed. The post-fix CI run shows Ruff Lint green.

Item 2 — Missing "already_deleted" not in json.dumps(err) pin

Added to test_remove_helpers_integrations_subentry_other_error_surfaces_service_call_failed (tests/src/unit/test_config_subentries_folded.py), symmetric with the string-form sibling test in the same file.

Diagnostic-hint pins on the raise branches

Added substring assertions on the user-facing error-message text for all six confirmed-absent raise-branch tests so the wording can't silently regress:

Test	Pins added
test_direct_path_entry_not_found_raises (Path 3)	"May indicate" + "ha_get_integration"
test_simple_path_state_gone_raises_entity_not_found (Path 1 state-gone)	"May indicate" + "ha_search_entities"
test_simple_path_404_on_state_check_raises_entity_not_found (Path 1 via 404)	same as state-gone sibling
test_flow_path_entity_not_in_registry_raises (Path 2 step 1)	"May indicate" + "ha_search_entities"
test_flow_path_entry_not_found_at_delete_raises (Path 2 TOCTOU)	"concurrent removal" (TOCTOU-specific framing)
test_remove_helpers_integrations_subentry_not_found_raises (Path 4)	"May indicate" + "ha_get_integration"

Path 2 TOCTOU keeps its distinct hint because semantically the entry WAS resolvable at step 1 — it only vanished before step 3 reached HA, so the typo framing wouldn't fit.

HAOS E2E (inaddon)

Agreed on flake — the failure was an httpx request-timeout during the CREATE step of test_forecast_solar_config_subentry_create_list_delete, before any Path 4 raise code is reached. The 9 prior runs on that workflow across other branches were green. The new CI run after this push is the empirical check.

Verification

76 unit tests in test_tools_integrations.py + test_config_subentries_folded.py pass locally. Ruff format clean on both files.

[kingpanther13]

All three items addressed cleanly in bd480611. Verified:

• Ruff format clean (CI green; commit 98f6ed2 landed before my prior review — my apologies for the cycle-noise on that one)
• "already_deleted" not in json.dumps(err) pin added to test_remove_helpers_integrations_subentry_other_error_surfaces_service_call_failed at line 565-566
• Diagnostic-hint substring pins added to all six raise-branch tests; cross-checked against the actual error messages at tools_integrations.py:1404, 1516, 1584, 1707, 1925 — wording matches
• Path 2 TOCTOU branch correctly gets distinct "concurrent removal" framing since the entry was resolvable at step 1 and only vanished before step 3

CI all green. Approving.

[kingpanther13]

@Patch76 sorry I didn't realize my e2e fix PR would conflict with yours, just ping me again once you fix the conflict and I will re-approve.

[Patch76]

No worries ...

[kingpanther13]

Re-approving after rebase. Verified squashed ce5539ca only resolves the conflict in test_config_entry_flow.py (absolute → relative imports, matching #1430's sweep) and picks up the upstream tools_utility.py changes from #1423. No new substantive changes beyond the prior approved state in bd480611. CI all green.

[github-actions]

🧪 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):
📖 Dev Channel Documentation

Quick start

# Run dev version
uvx ha-mcp-dev

# Check version
uvx ha-mcp-dev --version

Docker:

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:dev

---

Found an issue? Please open a new bug report and mention this PR for context.