Skip to content

Home Assistant: auto-detect Supervisor instance and token - #33000

Merged
andig merged 5 commits into
evcc-io:masterfrom
wlcrs:feature/home-assistant-supervisor-api
Aug 20, 2026
Merged

Home Assistant: auto-detect Supervisor instance and token#33000
andig merged 5 commits into
evcc-io:masterfrom
wlcrs:feature/home-assistant-supervisor-api

Conversation

@wlcrs

@wlcrs wlcrs commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

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_TOKEN environment variable will be set.

When this is the case, we call addInstance of the zeroconf-feature for HomeAssistant. This way it populates nicely as just another auto-discovered HomeAssistant in the UI.

@github-actions github-actions Bot added enhancement New feature or request devices Specific device support labels Aug 20, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread util/homeassistant/supervisor_test.go
Comment thread util/homeassistant/supervisor_test.go
@wlcrs

wlcrs commented Aug 20, 2026

Copy link
Copy Markdown
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
wlcrs marked this pull request as draft August 20, 2026 09:56
@wlcrs
wlcrs marked this pull request as ready for review August 20, 2026 10:58

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread util/homeassistant/supervisor_test.go Outdated
Comment thread util/homeassistant/supervisor.go Outdated
@andig

andig commented Aug 20, 2026

Copy link
Copy Markdown
Member

I can't assess if this is correct- happy to merge if it works in testing!

Co-authored-by: andig <cpuidle@gmail.com>
@wlcrs

wlcrs commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Happy to provide feedback once both PRs have landed in testing.

@andig
andig merged commit 6715711 into evcc-io:master Aug 20, 2026
11 checks passed
@wlcrs
wlcrs deleted the feature/home-assistant-supervisor-api branch August 20, 2026 11:41
@wlcrs

wlcrs commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

This works a charm. When testing with the nightly, I immediately am able to continue after selecting http://supervisor/core in the dropdown without needing to authenticate.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devices Specific device support enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants