Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Scrypted Home Assistant integration to avoid runtime-state collisions when multiple config entries end up with the same Scrypted bearer token, by keying active state by config entry ID and updating panel/proxy routing accordingly (while keeping legacy token URLs working).
Changes:
- Key
hass.data[DOMAIN]runtime state byconfig_entry.entry_id(storing bothentryandtoken) instead of keying by token. - Update the HTTP proxy view and panel URLs to use a stable
{identifier}(entry ID) while still accepting legacy token-based routes. - Add regression tests for duplicate entries sharing a token and for route compatibility.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
custom_components/scrypted/__init__.py |
Stores runtime state under entry ID; updates panel module URL and frontend path to entry ID; unload removes by entry ID. |
custom_components/scrypted/http.py |
Changes proxy route param to {identifier}; resolves identifier to runtime state (entry ID preferred, token fallback). |
custom_components/scrypted/sensor.py |
Looks up token from the new entry-id-keyed runtime state. |
tests/test_init.py |
Updates expectations for new runtime state shape; updates panel URL assertions; adds duplicate-token regression test. |
tests/test_sensor.py |
Updates test setup to match new runtime state layout. |
tests/test_http.py |
Adds coverage for resolving runtime state by entry ID and legacy token; validates entrypoint URL generation and URL creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @lru_cache | ||
| def _create_url(self, token: str, path: str) -> str: | ||
| def _create_url(self, identifier: str, path: str) -> str: | ||
| """Create URL to service.""" | ||
| entry: ConfigEntry = self.hass.data[DOMAIN][token] | ||
| entry: ConfigEntry = self._get_state(identifier)["entry"] | ||
| host = entry.data[CONF_HOST] | ||
| ipport = host.split(":") | ||
| if len(ipport) > 2: |
There was a problem hiding this comment.
_create_url is @lru_cached by (identifier, path), but identifier can be a legacy token that may correspond to multiple active entries. If two entries share a token and one is unloaded (or insertion order changes), cached URLs for that token can continue to point at the wrong entry/host. Consider removing the cache, or normalizing legacy-token requests to a unique cache key (e.g., resolve to state["entry"].entry_id first and cache on that).
| class ScryptedView(HomeAssistantView): | ||
| """Hass.io view to handle base part.""" | ||
|
|
||
| name = "api:scrypted" | ||
| url = "/api/scrypted/{token}/{path:.*}" | ||
| url = "/api/scrypted/{identifier}/{path:.*}" | ||
| requires_auth = False | ||
|
|
There was a problem hiding this comment.
This view is unauthenticated (requires_auth = False) and now uses a config entry ID as the primary route identifier. Since the server-side proxy uses the stored bearer token, anyone who learns/guesses the entry ID can proxy into Scrypted without HA auth. If this endpoint is intended to be protected by an unguessable secret, consider keeping a per-entry secret route ID (stored in the config entry) or continuing to use the bearer token for the unauthenticated route and only using entry_id for internal state keys.
| hass.data.setdefault(DOMAIN, {})[config_entry.entry_id] = { | ||
| "entry": config_entry, | ||
| "token": token, | ||
| } |
There was a problem hiding this comment.
Runtime state is now keyed by config_entry.entry_id, which prevents entry collisions, but the Lovelace card resources are still constructed from token (see _get_card_resource_definitions) and are registered/removed per entry. If two entries end up with the same token, they will share the same resource URLs and unloading one entry can remove resources still needed by the other. Consider moving the resource URLs to use entry_id (matching the new proxy route) or implementing reference-counting / “only remove if no other entry uses this token” logic.
| async_register_built_in_panel( | ||
| hass, | ||
| "custom", | ||
| sidebar_title=config_entry.data[CONF_NAME], | ||
| sidebar_icon=config_entry.data[CONF_ICON], | ||
| frontend_url_path=f"{DOMAIN}_{token}", | ||
| frontend_url_path=f"{DOMAIN}_{config_entry.entry_id}", | ||
| config=panelconf, | ||
| require_admin=False, | ||
| ) |
There was a problem hiding this comment.
async_unload_entry now removes the panel using entry_id (good for uniqueness), but the panel URL becomes stable and may be easier to leak than a backend-issued bearer token. Given the proxy endpoint is unauthenticated, consider whether the panel/frontend route should still be keyed by an unguessable secret (or require auth) to avoid exposing an unauthenticated proxy into Scrypted.
Summary
Testing