Add URL validation, graceful missing-device handling with real-time listeners, and editable merge sources in options flow#73
Merged
Conversation
…e merge devices Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/f2f108ac-e8e4-47d0-ba03-f536561f9bfa Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Validate configuration_url in config flow for device-tools
Add URL validation, graceful missing-device handling, and editable merge sources in options flow
Mar 30, 2026
Owner
|
@copilot I think you should not only check for missing/deleted devices/entities in |
… removal Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/69cde88c-685a-4c3a-b822-61736e637020 Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
Author
Done in commit
Both listeners are torn down in |
Copilot
AI
changed the title
Add URL validation, graceful missing-device handling, and editable merge sources in options flow
Add URL validation, graceful missing-device handling with real-time listeners, and editable merge sources in options flow
Mar 30, 2026
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the device_tools Home Assistant integration by adding user-input URL validation, making modifications resilient to deleted registry items (at startup and in real time), and allowing MERGE source devices to be edited in the options flow.
Changes:
- Add
configuration_urlvalidation in config flow + options flow with a newinvalid_configuration_urltranslation key. - Add startup-time cleanup/disable behavior for stale entity/device references and add engine-level registry “remove” listeners to react in real time.
- Allow editing MERGE source device IDs in options flow and recompute persisted merge original data accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
custom_components/device_tools/config_flow.py |
Adds URL validation helper, validates configuration_url in flows, and makes MERGE device selector editable + recomputes merge original data on options save. |
custom_components/device_tools/engine.py |
Adds startup-time stale-reference handling and registers engine-wide entity/device registry “remove” listeners with log-once behavior. |
custom_components/device_tools/translations/en.json |
Adds the new invalid_configuration_url error text and updates merge-source label to reflect editability. |
custom_components/device_tools/translations/de.json |
Adds the new invalid_configuration_url error text and updates merge-source label to reflect editability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+96
| """Return True if *value* is a valid absolute URL with a scheme and host.""" | ||
| try: | ||
| parsed = urllib.parse.urlparse(value) | ||
| return bool(parsed.scheme and parsed.netloc) |
There was a problem hiding this comment.
_is_valid_url() currently accepts any scheme as long as scheme and netloc are present, which would treat values like javascript://... as valid. Since configuration_url is typically rendered as a clickable link in the UI, please restrict allowed schemes (e.g., only http/https) to avoid potentially unsafe URL schemes being persisted.
Suggested change
| """Return True if *value* is a valid absolute URL with a scheme and host.""" | |
| try: | |
| parsed = urllib.parse.urlparse(value) | |
| return bool(parsed.scheme and parsed.netloc) | |
| """Return True if *value* is a valid absolute HTTP/HTTPS URL with a scheme and host.""" | |
| try: | |
| parsed = urllib.parse.urlparse(value) | |
| if not (parsed.scheme and parsed.netloc): | |
| return False | |
| return parsed.scheme.lower() in ("http", "https") |
Comment on lines
+117
to
+236
| mod_type = ModificationType(config_entry.data[CONF_MODIFICATION_TYPE]) | ||
| modification_entry_id: str | None = config_entry.data.get(CONF_MODIFICATION_ENTRY_ID) | ||
| modification_is_custom_entry: bool = config_entry.data.get( | ||
| CONF_MODIFICATION_IS_CUSTOM_ENTRY, False | ||
| ) | ||
| title: str = config_entry.title | ||
|
|
||
| entity_registry = er.async_get(self._hass) | ||
| device_registry = dr.async_get(self._hass) | ||
|
|
||
| # --- Graceful handling of deleted targets / sources --- | ||
|
|
||
| # ENTITY modification: check that the referenced entity still exists | ||
| if mod_type == ModificationType.ENTITY and modification_entry_id: | ||
| if entity_registry.async_get(modification_entry_id) is None: | ||
| warn_key = f"{config_entry.entry_id}_{modification_entry_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Entity %s referenced by modification '%s' no longer exists. " | ||
| "Disabling modification.", | ||
| modification_entry_id, | ||
| title, | ||
| ) | ||
| await self._hass.config_entries.async_set_disabled_by( | ||
| config_entry.entry_id, | ||
| ConfigEntryDisabler.USER, | ||
| ) | ||
| return | ||
|
|
||
| # DEVICE modification (non-creation): check that the referenced device still exists | ||
| if ( | ||
| mod_type == ModificationType.DEVICE | ||
| and not modification_is_custom_entry | ||
| and modification_entry_id | ||
| ): | ||
| if device_registry.async_get(modification_entry_id) is None: | ||
| warn_key = f"{config_entry.entry_id}_{modification_entry_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Device %s referenced by modification '%s' no longer exists. " | ||
| "Disabling modification.", | ||
| modification_entry_id, | ||
| title, | ||
| ) | ||
| await self._hass.config_entries.async_set_disabled_by( | ||
| config_entry.entry_id, | ||
| ConfigEntryDisabler.USER, | ||
| ) | ||
| return | ||
|
|
||
| # MERGE modification: remove any source devices that no longer exist | ||
| if mod_type == ModificationType.MERGE: | ||
| original_data: dict[str, Any] = config_entry.data.get( | ||
| CONF_MODIFICATION_ORIGINAL_DATA, {} | ||
| ) | ||
| missing_sources = [ | ||
| device_id | ||
| for device_id in original_data | ||
| if device_registry.async_get(device_id) is None | ||
| ] | ||
| if missing_sources: | ||
| new_original_data = { | ||
| k: v for k, v in original_data.items() if k not in missing_sources | ||
| } | ||
| for missing_id in missing_sources: | ||
| warn_key = f"{config_entry.entry_id}_{missing_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Device %s referenced by merge modification '%s' no longer " | ||
| "exists. Removing from merge sources.", | ||
| missing_id, | ||
| title, | ||
| ) | ||
| self._hass.config_entries.async_update_entry( | ||
| config_entry, | ||
| data={ | ||
| **config_entry.data, | ||
| CONF_MODIFICATION_ORIGINAL_DATA: new_original_data, | ||
| }, | ||
| ) | ||
| # config_entry is mutated in-place by HA; re-read data from it | ||
| config_entry = self._hass.config_entries.async_get_entry( | ||
| config_entry.entry_id | ||
| ) or config_entry | ||
|
|
||
| # DEVICE modification: remove assigned entities that no longer exist | ||
| if mod_type == ModificationType.DEVICE: | ||
| mod_data: dict[str, Any] = config_entry.options.get(CONF_MODIFICATION_DATA, {}) | ||
| assigned: list[str] = mod_data.get(CONF_ASSIGNED_ENTITIES, []) | ||
| missing_entities = [ | ||
| eid for eid in assigned if entity_registry.async_get(eid) is None | ||
| ] | ||
| if missing_entities: | ||
| new_assigned = [eid for eid in assigned if eid not in missing_entities] | ||
| for missing_id in missing_entities: | ||
| warn_key = f"{config_entry.entry_id}_{missing_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Entity %s assigned to device modification '%s' no longer " | ||
| "exists. Removing from assigned entities.", | ||
| missing_id, | ||
| title, | ||
| ) | ||
| new_mod_data = {**mod_data, CONF_ASSIGNED_ENTITIES: new_assigned} | ||
| new_options = { | ||
| **config_entry.options, | ||
| CONF_MODIFICATION_DATA: new_mod_data, | ||
| } | ||
| self._hass.config_entries.async_update_entry( | ||
| config_entry, | ||
| options=new_options, | ||
| ) | ||
| config_entry = self._hass.config_entries.async_get_entry( | ||
| config_entry.entry_id | ||
| ) or config_entry | ||
|
|
There was a problem hiding this comment.
The new startup-time cleanup/disabling logic for stale entity/device references changes engine behavior significantly, but there are currently no tests covering these cases in tests/test_engine.py. Please add tests that assert the correct config-entry updates (disable ENTITY/DEVICE entries; strip missing merge sources; remove missing assigned entities) and that handlers are not created/applied for disabled entries.
Comment on lines
+668
to
+791
| async def _async_on_entity_registry_updated( | ||
| self, | ||
| event: Event[er.EventEntityRegistryUpdatedData], | ||
| ) -> None: | ||
| """React to entity-registry events at the engine level. | ||
|
|
||
| Handles ``action == "remove"``: an entity that was the *target* of an | ||
| ENTITY modification is disabled; an entity that was *assigned* to a | ||
| DEVICE modification is removed from its ``CONF_ASSIGNED_ENTITIES`` list. | ||
| All other actions are ignored here (individual entity handlers react to | ||
| "update" actions themselves). | ||
| """ | ||
| if event.data["action"] != "remove": | ||
| return | ||
| removed_entity_id: str = event.data["entity_id"] | ||
|
|
||
| for entry in list(self._tracked_entries.values()): | ||
| mod_type = ModificationType(entry.data[CONF_MODIFICATION_TYPE]) | ||
| entry_target_id: str | None = entry.data.get(CONF_MODIFICATION_ENTRY_ID) | ||
|
|
||
| if mod_type == ModificationType.ENTITY and entry_target_id == removed_entity_id: | ||
| warn_key = f"{entry.entry_id}_{removed_entity_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Entity %s referenced by modification '%s' no longer exists. " | ||
| "Disabling modification.", | ||
| removed_entity_id, | ||
| entry.title, | ||
| ) | ||
| await self._hass.config_entries.async_set_disabled_by( | ||
| entry.entry_id, | ||
| ConfigEntryDisabler.USER, | ||
| ) | ||
|
|
||
| elif mod_type == ModificationType.DEVICE: | ||
| mod_data: dict[str, Any] = entry.options.get(CONF_MODIFICATION_DATA, {}) | ||
| assigned: list[str] = mod_data.get(CONF_ASSIGNED_ENTITIES, []) | ||
| if removed_entity_id in assigned: | ||
| new_assigned = [eid for eid in assigned if eid != removed_entity_id] | ||
| warn_key = f"{entry.entry_id}_{removed_entity_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Entity %s assigned to device modification '%s' no longer " | ||
| "exists. Removing from assigned entities.", | ||
| removed_entity_id, | ||
| entry.title, | ||
| ) | ||
| new_mod_data = {**mod_data, CONF_ASSIGNED_ENTITIES: new_assigned} | ||
| self._hass.config_entries.async_update_entry( | ||
| entry, | ||
| options={**entry.options, CONF_MODIFICATION_DATA: new_mod_data}, | ||
| ) | ||
|
|
||
| async def _async_on_device_registry_updated( | ||
| self, | ||
| event: Event[dr.EventDeviceRegistryUpdatedData], | ||
| ) -> None: | ||
| """React to device-registry events at the engine level. | ||
|
|
||
| Handles ``action == "remove"``: a device that was the *target* of a | ||
| non-creation DEVICE modification is disabled; a device that was a | ||
| *source* in a MERGE modification is removed from | ||
| ``CONF_MODIFICATION_ORIGINAL_DATA``. | ||
| All other actions are ignored here (individual device handlers react | ||
| to "update" actions themselves). | ||
| """ | ||
| if event.data["action"] != "remove": | ||
| return | ||
| removed_device_id: str = event.data["device_id"] | ||
|
|
||
| for entry in list(self._tracked_entries.values()): | ||
| mod_type = ModificationType(entry.data[CONF_MODIFICATION_TYPE]) | ||
| entry_target_id: str | None = entry.data.get(CONF_MODIFICATION_ENTRY_ID) | ||
| modification_is_custom_entry: bool = entry.data.get( | ||
| CONF_MODIFICATION_IS_CUSTOM_ENTRY, False | ||
| ) | ||
|
|
||
| if ( | ||
| mod_type == ModificationType.DEVICE | ||
| and not modification_is_custom_entry | ||
| and entry_target_id == removed_device_id | ||
| ): | ||
| warn_key = f"{entry.entry_id}_{removed_device_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Device %s referenced by modification '%s' no longer exists. " | ||
| "Disabling modification.", | ||
| removed_device_id, | ||
| entry.title, | ||
| ) | ||
| await self._hass.config_entries.async_set_disabled_by( | ||
| entry.entry_id, | ||
| ConfigEntryDisabler.USER, | ||
| ) | ||
|
|
||
| elif mod_type == ModificationType.MERGE: | ||
| original_data: dict[str, Any] = entry.data.get( | ||
| CONF_MODIFICATION_ORIGINAL_DATA, {} | ||
| ) | ||
| if removed_device_id in original_data: | ||
| new_original_data = { | ||
| k: v | ||
| for k, v in original_data.items() | ||
| if k != removed_device_id | ||
| } | ||
| warn_key = f"{entry.entry_id}_{removed_device_id}" | ||
| if warn_key not in self._warned_missing: | ||
| self._warned_missing.add(warn_key) | ||
| _LOGGER.warning( | ||
| "Device %s referenced by merge modification '%s' no longer " | ||
| "exists. Removing from merge sources.", | ||
| removed_device_id, | ||
| entry.title, | ||
| ) | ||
| self._hass.config_entries.async_update_entry( | ||
| entry, | ||
| data={ | ||
| **entry.data, | ||
| CONF_MODIFICATION_ORIGINAL_DATA: new_original_data, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
The new engine-level registry "remove" listeners update/disable config entries in response to entity/device deletions, but there are no tests verifying these real-time behaviors. Please add tests that fire EVENT_ENTITY_REGISTRY_UPDATED / EVENT_DEVICE_REGISTRY_UPDATED with action == "remove" and assert the expected config entry updates (disable target mods, strip assigned entities, remove merge sources) and that the engine reacts without errors.
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.
Three improvements to the
device_toolsintegration: config-flow URL validation, graceful degradation when referenced devices/entities are deleted (both at startup and in real time), and the ability to edit merge source devices after creation.URL validation for
configuration_urlasync_step_modify_deviceand options-flowasync_step_initnow validateconfiguration_urlusingurllib.parse.urlparsebefore saving. Invalid values re-show the form with{"base": "invalid_configuration_url"}instead of propagating an HA error. Translation key added to bothen.jsonandde.json.Graceful handling of deleted registry entries (
engine.py)At startup (
async_on_entry_loaded)Stale references are detected before setting up handlers:
async_set_disabled_byand skips handler setup.CONF_MODIFICATION_ORIGINAL_DATAand continues with the remaining ones.CONF_ASSIGNED_ENTITIES.In real time (registry deletion listeners)
Two engine-level bus listeners are registered in
async_startand torn down inasync_stop:_async_on_entity_registry_updated— onaction == "remove": disables any ENTITY modification whose target was the removed entity; strips the entity fromCONF_ASSIGNED_ENTITIESof any DEVICE modification that referenced it._async_on_device_registry_updated— onaction == "remove": disables any non-creation DEVICE modification whose target was the removed device; removes the device fromCONF_MODIFICATION_ORIGINAL_DATAof any MERGE modification that listed it as a source.All warnings are gated by a
_warned_missing: set[str](keyedentry_id + "_" + missing_id) so each event logs at most once per session.Editable merge-source devices in options flow
read_only=Truefrom theCONF_MERGE_DEVICE_IDSDeviceSelector.async_step_initMERGE branch now recomputesCONF_MODIFICATION_ORIGINAL_DATA: retained devices keep their existing snapshot; newly-added devices have their entity data gathered fresh from the entity registry. The config entry data is updated viaasync_update_entry."Devices to merge (cannot be changed currently)"updated in both locale files.Original prompt
Overview
Three improvements need to be implemented in the
EuleMitKeule/device-toolsHome Assistant custom integration. The target branch isdev-2.x. After implementing all changes, ensure the CI tools pass: ruff, mypy (strict, againstcustom_components/device_tools), and pytest (undertests/). The CI commands are:Task 1: Validate
configuration_urlin the config flowProblem: When a user enters a value for
configuration_urlthat is not a valid URL, Home Assistant raises an error. We need to catch this and show a proper validation error in the form instead.What to do:
custom_components/device_tools/config_flow.py, in both the config flow stepasync_step_modify_deviceand the options flowasync_step_init, add validation ofCONF_CONFIGURATION_URLwhen it is present in the processed user input.voluptuous'svol.Url()validator or Python'surllib.parse.urlparseto check it's a valid URL.errorsdict like{"base": "invalid_configuration_url"}."invalid_configuration_url"tocustom_components/device_tools/translations/en.jsonandcustom_components/device_tools/translations/de.jsonunder theerrorssection.Task 2: Gracefully handle deleted devices/entities referenced by modifications
Problem: If a device or entity referenced by a modification no longer exists in the registry, the integration can behave unexpectedly. We need to detect this at startup and handle it gracefully.
What to do in
custom_components/device_tools/engine.py(ModificationEngine.async_on_entry_loaded):For ENTITY modifications where
CONF_MODIFICATION_ENTRY_IDreferences a non-existent entity:"Entity <id> referenced by modification '<title>' no longer exists. Disabling modification."self._hass.config_entries.async_update_entry(config_entry, disabled_by=ConfigEntryDisabledBy.USER)— importConfigEntryDisabledByfromhomeassistant.config_entries.For DEVICE modifications (non-creation, i.e.
modification_is_custom_entry=False) whereCONF_MODIFICATION_ENTRY_IDreferences a non-existent device:"Device <id> referenced by modification '<title>' no longer exists. Disabling modification."For MERGE modifications where a source device in
CONF_MODIFICATION_ORIGINAL_DATAno longer exists:"Device <id> referenced by merge modification '<title>' no longer exists. Removing from merge sources."CONF_MODIFICATION_ORIGINAL_DATAusingself._hass.config_entries.async_update_entry(config_entry, data={...}).For DEVICE modifications where
CONF_ASSIGNED_ENTITIEScontains entity IDs that don't exist:"Entity <id> assigned to device modification '<title>' no longer exists. Removing from assigned entities."CONF_ASSIGNED_ENTITIES.Log-once behavior: To avoid spamming the log on every restart, track already-warned entries in a set
self._warned_missing: set[str](keyed byconfig_entry.entry_id + "_" + missing_id). Only log if that key is not already in the set.Task 3: Allow editing merge device IDs in the options flow
Problem: The options flow for MERGE modifications shows
CONF_MERGE_DEVICE_IDSas read-only, so users cannot change which devices are merged.What to do:
In
_get_merge_options_schemainconfig_flow.py: Removeread_only=Truefrom theDeviceSelectorforCONF_MERGE_DEVICE_IDS.In the options flow
async_step_init: Whenmodification_type == ModificationType.MERGEand the user submits the form:user_input[CONF_MERGE_OPTIONS][CONF_MERGE_DEVICE_IDS].async_step_merge_device).CONF_MODIFICATION_ORIGINAL_DATA: for newly-added device IDs, gather their entities from the entity registry (same logic as inasync_step_merge_device). For removed device IDs, drop them.CONF_MODIFICATION_ORIGINAL_DATAusingself.hass.config_entries.async_update_entry(self.config_entry, data={...new_data...}).modification_data(the options) appropriately.Relevant code
config_flow.py_get_merge_options_schema(lines 431-468):