Use aiohasupervisor for all Supervisor service calls#166558
Conversation
|
Hey there @home-assistant/supervisor, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This PR completes the aiohasupervisor migration for the hassio integration by moving Supervisor service calls to use SupervisorClient methods instead of dynamically constructed HTTP endpoints.
Changes:
- Introduces a dedicated
homeassistant/components/hassio/services.pyto register and implement Supervisor services viaaiohasupervisor. - Updates backup services to optionally return the newly created backup slug via service responses.
- Tightens partial backup/restore validation to reject duplicate
addons/appsandfolders, and updates tests accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/hassio/services.py |
New module implementing Supervisor services using SupervisorClient, including optional service responses for backup creation. |
homeassistant/components/hassio/__init__.py |
Removes the legacy dynamic service→endpoint mapping and delegates registration to the new services module. |
tests/components/hassio/test_init.py |
Updates service-call tests to assert SupervisorClient method usage and adds tests for duplicate validation. |
homeassistant/components/hassio/websocket_api.py |
Adjusts HassioAPIError import location. |
homeassistant/components/hassio/addon_manager.py |
Simplifies error handling to only catch SupervisorError now that calls go through aiohasupervisor. |
| async def async_app_stdin_service_handler(service: ServiceCall) -> None: | ||
| """Handles app stdin service.""" | ||
| app_slug = service.data[ATTR_APP] | ||
| data: dict | str = service.data[ATTR_INPUT] | ||
|
|
||
| if isinstance(data, dict): | ||
| data = json.dumps(data) | ||
| payload = data.encode(encoding="utf-8") | ||
|
|
||
| try: | ||
| await supervisor_client.addons.write_addon_stdin(app_slug, payload) | ||
| except SupervisorError as err: | ||
| raise HomeAssistantError( | ||
| f"Failed to write stdin to app {app_slug}: {err}" | ||
| ) from err | ||
|
|
||
| hass.services.async_register( | ||
| DOMAIN, | ||
| SERVICE_APP_STDIN, | ||
| async_app_stdin_service_handler, | ||
| schema=SCHEMA_APP_STDIN, | ||
| ) |
There was a problem hiding this comment.
So this one is a bit different from before. Here's the old code to compare:
core/homeassistant/components/hassio/__init__.py
Lines 237 to 239 in f84398e
core/homeassistant/components/hassio/__init__.py
Lines 513 to 539 in f84398e
core/homeassistant/components/hassio/handler.py
Lines 51 to 89 in f84398e
It appears before we allowed input to be a dictionary or a string. And then we took that dictionary or string and handed it off to aiohttp as json to make a request?
The problem is Supervisor is looking for binary input for this API, it's going to simply read the request as bytes and pipe it directly to the container. So I'm not entirely sure how this was working but it seems like a mess of type and encoding issues.
I adjusted it to encode whatever we're given as input so I can then hand bytes to the library like it (and Supervisor) expects. I'm worried this will create a change in behavior though because I really don't know how Supervisor handles it when an API that is expecting an application/octet-stream is called with an application/json request body. It must've worked a fair amount of the time but there's probably a lot of edge cases that aren't accounted for?
There was a problem hiding this comment.
Since we were passing string values provided as input as a json payload to aiohttp I bet it was coming through in quotes to supervisor 🤔
I'll have to test that. I might have to add quoted around it before encoding it for backwards compatibility 🙈
There was a problem hiding this comment.
Ok I made a dumb addon that just accepts the stdin service input, reads it and logs it. I sent it a few tests. Here's the entirety of the addon's runtime logic:
#!/usr/bin/with-contenv bashio
set -e
# Read from STDIN aliases to send shutdown
while read -r input; do
bashio::log.info "Input: $input"
done- Example input 1:
action: hassio.addon_stdin
data:
addon: local_example_input
input: "test"Result:
[11:33:55] INFO: Input: "test"
- Example input 2:
action: hassio.addon_stdin
data:
addon: local_example_input
input:
a: a
b: bResult:
[11:34:16] INFO: Input: {"a":"a","b":"b"}
- Example input 3:
action: hassio.addon_stdin
data:
addon: local_example_input
input: "🥹"Result:
[11:44:46] INFO: Input: "🥹"
So this poses a bit of an issue. I do have to wrap the strings provided in quotes for backwards compatibility as expected. But the encoding bit is even more of a challenge. I can't encode to bytes without knowing the encoding and it appears non-utf-8 worked fine.
I'm not entirely sure what to do with this service given this. Maybe I add new raw_input and encoding config options and deprecate input? And then have any usage of input continuing working exactly as it does today going through HassIO.send_command and the new things go through the library until the deprecation period ends. Unless we can come up with a way to make this behave sensibly and be backwards compatible?
There was a problem hiding this comment.
A solution that I thought of was to do the JSON encoding ourselves first and then encode that JSON string to bytes. Would that work?
Otherwise it also sounds ok to change the service action parameters as suggested and a deprecation.
There was a problem hiding this comment.
It looks like you already did my suggestion.
There was a problem hiding this comment.
@MartinHjelmare yea I guess this works. For some reason I thought we'd have an encoding problem but I guess we don't really. We just add a note that its utf-8 encoded, which was already true since its the default and we didn't specify.
It does feel like they should have a way to provide text as input that gets used as is without being wrapped in quotes though. But perhaps that's just a separate enhancement.
There was a problem hiding this comment.
Yeah, I think it's fine to keep it working as it did, for now.
There was a problem hiding this comment.
I agree, preserving how things worked today make sense. We probably should add input_raw or something down the line.
FWIW, I checked how the rpc_shutdown app handles the quoted input: The app uses bashio::jq on the stdin. bashio::jq uses --raw-output, which strips the quotes, converting the json string to an unquoted string. If we were to send a unquoted string, rpc_shutdown would stop working since jq expects valid json (and unquoted string is not valid json).
It probably make sense to document the behvior in the hassio.app_stdin action docs.
| return value | ||
|
|
||
|
|
||
| SCHEMA_NO_DATA = vol.Schema({}) |
There was a problem hiding this comment.
I'm assuming all these schemas are just moved and not changed otherwise?
There was a problem hiding this comment.
@MartinHjelmare mostly. As mentioned at the top there's changes to the partial backup and restore schemas, I changed the schema for the addons, apps and folders fields to require each item in the list be unique. Because the client library wants a set not a list. And also because supervisor requires those have unique values or it rejects the request with a 400 response (not new behavior, this is how it has always worked).
That was why I asked about that at the top in the PR description. If a user had a service call like these in their automations:
- action: hassio.partial_backup
data:
folders:
- ssl
- share
- ssl
- action: hassio.partial_restore
data:
slug: my_backup
apps:
- core_ssh
- core_samba
- core_sshThen they would've failed at runtime. Supervisor would've responded with a 400 and unless they had continue_on_error: true the automation/script would've ended. But it would've saved and been considered valid config. With my change to the schema this will be marked as invalid config.
That's why I asked at the top if this is a considered a breaking change or not. And if so, if it should be separated into a different PR.
There was a problem hiding this comment.
Thanks for reminding me. I had read that earlier. I think those changes to the schemas are fine since it wouldn't have worked anyway. We're just failing earlier which is good.
|
Besides the topic of discussion above, the code looks ok. 👍 |
4821e85 to
db56157
Compare
agners
left a comment
There was a problem hiding this comment.
- Supervisor's partial backup and partial restore APIs have always been set to reject calls as invalid if the list of
addonsorfolderscontained duplicates. This was not captured in the schema for these service calls. I updated them to raise invalid in this case. I'm not sure if this qualifies as a breaking change though? Since the call would've failed I figured no but it would've at least saved before and now it won't.
So I've tested that, before you really didn't get any feedback at all it seems, not even in logs. it just didn't work. With this change, we get a proper error, but only on execution (it seems that validation only happens at action execution time, not safe!)
2026-03-30 15:25:31.692 INFO (MainThread) [homeassistant.components.automation.neue_automation] Neue Automation: Running automation actions
2026-03-30 15:25:31.692 INFO (MainThread) [homeassistant.components.automation.neue_automation] Neue Automation: Executing step call service
2026-03-30 15:25:31.693 ERROR (MainThread) [homeassistant.components.automation.neue_automation] Neue Automation: Error executing script. Invalid data for call_service at pos 1: two or more values in the same group of exclusion 'apps_or_addons' @ data[<apps_or_addons>]
2026-03-30 15:25:31.693 ERROR (MainThread) [homeassistant.components.automation.neue_automation] Error while executing automation automation.neue_automation: two or more values in the same group of exclusion 'apps_or_addons' @ data[<apps_or_addons>]
So the new behavior is much better. It also isn't a problem if existing automations have that error, they will still safe, but now have better logs.
9732324 to
f74256d
Compare
Breaking change
Previously all actions registered by the supervisor integration (
hassio.app_start,hassio.backup_partial,hassio.host_reboot, etc.) simply logged an error on failure. The script/automation proceeded regardless of whether that action succeeded. Now they properly raise on failure which means the automation/script will stop unlesscontinue_on_erroris set toTrue.Proposed change
Final task in the long-running
aiohasupervisormigration - the service calls. These were implemented in a dynamic way but they are static and known at design time so re-implemented them to use aiohasupervisor for all communication with Supervisor.As the service calls had not been updated in a while, made two small changes. Let me know if one or both of these belong in a different PR and I can break them out:
addonsorfolderscontained duplicates. This was not captured in the schema for these service calls. I updated them to raise invalid in this case. I'm not sure if this qualifies as a breaking change though? Since the call would've failed I figured no but it would've at least saved before and now it won't.Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: