Home Assistant: auto-detect Supervisor instance and token - #33000
Merged
andig merged 5 commits intoAug 20, 2026
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The supervisor-specific logic hinges on an exact string match to
SupervisorURI; consider allowing equivalent URIs (e.g., different schemes/hosts or configurable base URL) so a valid supervisor setup isn’t accidentally bypassed due to minor URI differences. - The
initinsupervisor.goadds an instance based on the environment at process start; if the token or URI can change at runtime, it may be preferable to trigger discovery lazily when first needed rather than ininit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The supervisor-specific logic hinges on an exact string match to `SupervisorURI`; consider allowing equivalent URIs (e.g., different schemes/hosts or configurable base URL) so a valid supervisor setup isn’t accidentally bypassed due to minor URI differences.
- The `init` in `supervisor.go` adds an instance based on the environment at process start; if the token or URI can change at runtime, it may be preferable to trigger discovery lazily when first needed rather than in `init`.
## Individual Comments
### Comment 1
<location path="util/homeassistant/supervisor_test.go" line_range="16-21" />
<code_context>
+func TestSupervisorToken(t *testing.T) {
+ t.Setenv(SupervisorToken, "test_supervisor_token")
+
+ ts, ok := supervisorTokenSource(SupervisorURI)
+ require.True(t, ok)
+
+ tok, err := ts.Token()
+ require.NoError(t, err)
+ assert.Equal(t, "test_supervisor_token", tok.AccessToken)
+
+ // empty uri does not match supervisor
+ _, ok = supervisorTokenSource("")
+ assert.False(t, ok)
+
+ // other uri does not match supervisor
+ _, ok = supervisorTokenSource("http://homeassistant.local:8123")
+ assert.False(t, ok)
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case for `SupervisorURI` variants with trailing slash to cover `strings.TrimRight` logic
`supervisorTokenSource` strips trailing slashes via `strings.TrimRight(uri, "/")`, but the test only covers `SupervisorURI` without a trailing slash. Please add a case verifying that `supervisorTokenSource("http://supervisor/core/")` still returns a valid token source so this normalization behavior is covered and protected against regressions.
```suggestion
ts, ok := supervisorTokenSource(SupervisorURI)
require.True(t, ok)
tok, err := ts.Token()
require.NoError(t, err)
assert.Equal(t, "test_supervisor_token", tok.AccessToken)
// SupervisorURI variant with trailing slash should still match
tsSlash, ok := supervisorTokenSource(SupervisorURI + "/")
require.True(t, ok)
tokSlash, err := tsSlash.Token()
require.NoError(t, err)
assert.Equal(t, "test_supervisor_token", tokSlash.AccessToken)
```
</issue_to_address>
### Comment 2
<location path="util/homeassistant/supervisor_test.go" line_range="31-37" />
<code_context>
+ _, ok = supervisorTokenSource("http://homeassistant.local:8123")
+ assert.False(t, ok)
+
+ // from config with SupervisorURI
+ ts3, err := NewHomeAssistantFromConfig(map[string]any{"uri": SupervisorURI})
+ require.NoError(t, err)
+
+ tok3, err := ts3.Token()
+ require.NoError(t, err)
+ assert.Equal(t, "test_supervisor_token", tok3.AccessToken)
+
+ // connection requires uri
</code_context>
<issue_to_address>
**suggestion (testing):** Add a negative test for `NewHomeAssistantFromConfig` when `SUPERVISOR_TOKEN` is unset to ensure fallback to OAuth
You currently only test the case where a supervisor token is present. Please add a test where `SUPERVISOR_TOKEN` is unset (e.g. `t.Setenv(SupervisorToken, "")`) to confirm `NewHomeAssistantFromConfig` falls back to the standard OAuth token source and does not attempt to use a supervisor token.
Suggested implementation:
```golang
// from config with SupervisorURI
ts3, err := NewHomeAssistantFromConfig(map[string]any{"uri": SupervisorURI})
require.NoError(t, err)
tok3, err := ts3.Token()
require.NoError(t, err)
assert.Equal(t, "test_supervisor_token", tok3.AccessToken)
// when SUPERVISOR_TOKEN is unset, NewHomeAssistantFromConfig should fall back to the standard OAuth token source
t.Setenv(SupervisorToken, "")
ts4, err := NewHomeAssistantFromConfig(map[string]any{"uri": SupervisorURI})
require.NoError(t, err)
tok4, err := ts4.Token()
require.NoError(t, err)
// ensure we did not use the supervisor token source
assert.NotEqual(t, "test_supervisor_token", tok4.AccessToken)
// connection requires uri
```
If your OAuth path in tests returns a specific, known token (for example a mocked `"test_oauth_token"`), you can strengthen the assertion by changing `assert.NotEqual` to `assert.Equal` against that known value:
- Replace `assert.NotEqual(t, "test_supervisor_token", tok4.AccessToken)` with `assert.Equal(t, "test_oauth_token", tok4.AccessToken)` (or whatever token the OAuth mock produces).
Also ensure that `SupervisorToken` is the correct environment variable name used by `NewHomeAssistantFromConfig` for the supervisor token; if it differs, adjust the `t.Setenv` call accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
URI endpoint are static, so we don't need to support other values. SUPERVISOR_TOKEN doesn't change once the container has started, so the current approach is valid. |
wlcrs
marked this pull request as draft
August 20, 2026 09:56
wlcrs
marked this pull request as ready for review
August 20, 2026 10:58
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
supervisorTokenSourcehelper currently only activates when the URI exactly matchesSupervisorURI; if users configure Home Assistant with an external URL or different base path, consider whether the Supervisor token should still be leveraged or make this constraint explicit in code (e.g., via comments or validation). - The
initinsupervisor.gocallsaddInstancewhenever a supervisor token is present; if multiple configuration paths can also add the same instance, it may be worth guarding against duplicate registrations or normalizing how instances are deduplicated.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `supervisorTokenSource` helper currently only activates when the URI exactly matches `SupervisorURI`; if users configure Home Assistant with an external URL or different base path, consider whether the Supervisor token should still be leveraged or make this constraint explicit in code (e.g., via comments or validation).
- The `init` in `supervisor.go` calls `addInstance` whenever a supervisor token is present; if multiple configuration paths can also add the same instance, it may be worth guarding against duplicate registrations or normalizing how instances are deduplicated.
## Individual Comments
### Comment 1
<location path="util/homeassistant/supervisor_test.go" line_range="53-55" />
<code_context>
+ ts4, err := NewHomeAssistantFromConfig(map[string]any{"uri": SupervisorURI})
+ require.NoError(t, err)
+
+ _, err = ts4.Token()
+ require.Error(t, err)
+ assert.ErrorContains(t, err, "login required")
+
+ // connection requires uri
</code_context>
<issue_to_address>
**suggestion (testing):** Avoid asserting on specific error message text for the OAuth fallback case
Asserting on the exact string "login required" makes this test fragile if the message wording changes but the behavior stays correct. Prefer asserting on the error type or other stable signal (e.g., that an error is returned from the OAuth path). If there’s a sentinel error or specific classification for this case, use that instead to keep the test maintainable.
Suggested implementation:
```golang
ts4, err := NewHomeAssistantFromConfig(map[string]any{"uri": SupervisorURI})
require.NoError(t, err)
_, err = ts4.Token()
require.Error(t, err)
// use a stable sentinel error instead of fragile message matching
assert.ErrorIs(t, err, ErrLoginRequired)
```
1. Ensure there is a package-level sentinel error (e.g. `var ErrLoginRequired = errors.New("login required")`) defined in the Home Assistant/supervisor implementation, and that the OAuth fallback path returns or wraps this error.
2. If an existing error type or sentinel already represents the "login required" condition, replace `ErrLoginRequired` in the test with that existing identifier.
3. If the project prefers classification methods (e.g. `IsLoginRequired(err)`), then use `assert.True(t, IsLoginRequired(err))` instead of `assert.ErrorIs`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
andig
reviewed
Aug 20, 2026
Member
|
I can't assess if this is correct- happy to merge if it works in testing! |
Co-authored-by: andig <cpuidle@gmail.com>
Contributor
Author
|
Happy to provide feedback once both PRs have landed in testing. |
Contributor
Author
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.

This is the companion PR to evcc-io/hassio-addon#142 .
It automatically discovers the Home Assistant Core API endpoint and authenticates with the injected bearer token when evcc runs as a Home Assistant add-on, where after merging evcc-io/hassio-addon#142 the
SUPERVISOR_TOKENenvironment variable will be set.When this is the case, we call
addInstanceof the zeroconf-feature for HomeAssistant. This way it populates nicely as just another auto-discovered HomeAssistant in the UI.