Skip to content

Commit 98fc499

Browse files
authored
feat: add snapshot delete + fix backup progress heartbeats (#1861) (#1881)
* feat: add snapshot delete + fix backup progress heartbeats (#1861) - ha_manage_backup(scope='snapshot', action='create'/'restore') now emits MCP progress heartbeats during the wait so clients don't idle-abort a long-running backup; raised the internal poll ceiling 300s -> 1800s (HA's own frontend imposes no timeout at all on backup creation) - New (scope='snapshot', action='delete') action, off by default via enable_snapshot_delete, layered with guards even when enabled: requires confirm=True, never deletes scheduled/automatic backups, enforces a snapshot_delete_min_age_days floor (default 7, an HA-stamped date an agent can't forge), and never deletes the single newest snapshot remaining - Wires the two new settings through both HA add-on flavors (config.yaml + start.py) and the web settings UI - Unit + e2e coverage, including a real create->delete->verify-gone round trip against a live HA test instance * fix: add missing translations for the two new backup settings CI caught it: test_translations_cover_every_schema_key requires every config.yaml schema key to have a matching translations/en.yaml entry. enable_snapshot_delete and snapshot_delete_min_age_days were wired into config.yaml + start.py but the translations file was missed, in both add-on flavors. * fix: guard against a null result key in backup/info + backup/delete responses Gemini Code Assist review: info_result.get("result", {}) and delete_result.get("result", {}) only substitute the default when the key is ABSENT, not when it's present with value None — a well-formed-but-null "result" would raise AttributeError on the following .get() call. Switch to the (x.get("result") or {}) pattern already used elsewhere in this file (_build_success_response_if_found, _poll_backup_completion). * fix: wire enable_snapshot_delete through the embedded/HAOS e2e backends The container-embedded (E2E_BACKEND=embedded) and HAOS embedded/inaddon backends run ha-mcp inside the HA instance itself, in a separate process from pytest — os.environ[...] set in the pytest process, and monkeypatch.setenv + _reset_global_settings(), never reach it. Both need their own delivery mechanism: - container-embedded: seed backup_settings.json (BACKUP_OVERRIDE_FIELDS) in the embedded server's data dir, alongside the existing feature_flags.json (FEATURE_FLAG_FIELDS) — two different override files since ha_mcp.config reads the two registries separately. - HAOS embedded/inaddon (qcow2): generalize stage_embedded_server_feature_flags_in_qcow2 to take a filename, and call it a second time for backup_settings.json. Also skip the age-floor-disabled success-path e2e test under all three out-of-process backends — the guard-rejection tests already exercise the real delete path end-to-end there; only the "actually completes" happy-path needs live settings mutation from the test process. * fix: three codex-review gaps in snapshot delete - Refuse deletion when backup/info reports agent_errors: an unreachable agent means the backups list may be incomplete, so the newest-snapshot and scheduled-backup guards can't be trusted. - min_age_days=0 now unconditionally skips the age comparison instead of relying on the arithmetic reduction, which broke under clock skew between the ha-mcp host and the HA instance that stamped the date. - backup/delete now passes an elevated _wait_timeout (60s vs the 30s client default): it fans out to every configured agent server-side, including slow/rate-limited remote ones, before responding. * fix: skip TestSnapshotDelete on the HAOS inaddon e2e tier The inaddon tier runs the real dev add-on against its own Supervisor-managed options.json (config.yaml's shipped enable_snapshot_delete=false default) — unlike container-embedded / HAOS-embedded, which get the setting via a seeded backup_settings.json override file, there is no in-process server here to seed a file for (overriding would mean POSTing to the real Supervisor options API). Mirrors the existing skip precedent in tests/src/e2e/policy/test_readonly_mode.py for settings that can't be forced under this tier. * fix: address kingpanther13 review on snapshot delete - Fail closed when with_automatic_settings is None, not just True - Pin newest-before-age-floor evaluation order with a regression test - Wire ctx heartbeat into delete_backup's WS wait - Pin heartbeat throttle behavior with a regression test - Fix docstring/comment ordering-accuracy nits
1 parent 3e13a71 commit 98fc499

19 files changed

Lines changed: 1607 additions & 41 deletions

File tree

homeassistant-addon-dev/config.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ options:
5757
enable_auto_backup: true
5858
auto_backup_throttle_minutes: 0
5959
auto_backup_retain_per_entity: 100
60+
# Off by default (#1861) — an agent deleting a full HA snapshot is
61+
# categorically riskier than the lightweight edits-scope auto-backups
62+
# above (a snapshot may be the last recovery point after the agent
63+
# itself broke something). A human opts in here, not the agent.
64+
enable_snapshot_delete: false
65+
snapshot_delete_min_age_days: 7
6066
tool_search_max_results: 5
6167
disabled_tools: ""
6268
pinned_tools: ""
@@ -82,6 +88,8 @@ schema:
8288
enable_auto_backup: bool?
8389
auto_backup_throttle_minutes: int(0,1440)?
8490
auto_backup_retain_per_entity: int(1,10000)?
91+
enable_snapshot_delete: bool?
92+
snapshot_delete_min_age_days: int(0,365)?
8593
tool_search_max_results: int(2,10)?
8694
disabled_tools: str?
8795
pinned_tools: str?

homeassistant-addon-dev/translations/en.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,21 @@ configuration:
156156
Maximum number of snapshots kept per entity. Older snapshots beyond
157157
this cap are rotated out on each successful capture. Default 100,
158158
range 1–10000.
159+
enable_snapshot_delete:
160+
name: Allow snapshot deletion
161+
description: >-
162+
Lets ha_manage_backup delete full HA snapshot tarballs
163+
(scope='snapshot', action='delete'). Off by default — a snapshot may
164+
be the last recovery point after a mistaken change, so a human must
165+
opt in here. Even when on, scheduled backups, the newest remaining
166+
snapshot, and anything younger than the age floor below stay
167+
protected.
168+
snapshot_delete_min_age_days:
169+
name: Minimum snapshot age to delete (days)
170+
description: >-
171+
A snapshot must be at least this old before it can be deleted. Range
172+
0–365; 0 disables the floor (the newest-snapshot and
173+
scheduled-backup protections still apply). Default 7.
159174
enable_lite_docstrings:
160175
name: Enable lite tool docstrings (beta)
161176
description: >-

homeassistant-addon/config.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ options:
5555
enable_auto_backup: true
5656
auto_backup_throttle_minutes: 0
5757
auto_backup_retain_per_entity: 100
58+
# Off by default (#1861) — an agent deleting a full HA snapshot is
59+
# categorically riskier than the lightweight edits-scope auto-backups
60+
# above (a snapshot may be the last recovery point after the agent
61+
# itself broke something). A human opts in here, not the agent.
62+
enable_snapshot_delete: false
63+
snapshot_delete_min_age_days: 7
5864
verify_ssl: true
5965
schema:
6066
backup_hint: list(strong|normal|weak|auto)
@@ -70,6 +76,8 @@ schema:
7076
enable_auto_backup: bool?
7177
auto_backup_throttle_minutes: int(0,1440)?
7278
auto_backup_retain_per_entity: int(1,10000)?
79+
enable_snapshot_delete: bool?
80+
snapshot_delete_min_age_days: int(0,365)?
7381
verify_ssl: bool?
7482
# Add-on exposes HTTP port for MCP communication (fixed internal port)
7583
ports:

homeassistant-addon/start.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ def main() -> int:
383383
)
384384
auto_backup_throttle_minutes = 0 # default — every write
385385
auto_backup_retain_per_entity = 100 # default
386+
# Off by default (#1861 — a snapshot may be the last recovery point
387+
# after the agent itself broke something; a human opts in, not the
388+
# agent).
389+
enable_snapshot_delete = False # default
390+
snapshot_delete_min_age_days = 7 # default
386391
tool_search_max_results = 5 # default
387392
disabled_tools_raw = "" # default
388393
pinned_tools_raw = "" # default
@@ -518,6 +523,14 @@ def main() -> int:
518523
auto_backup_retain_per_entity = (
519524
raw_retain if isinstance(raw_retain, int) else 100
520525
)
526+
raw_snapshot_delete = config.get("enable_snapshot_delete", False)
527+
enable_snapshot_delete = (
528+
raw_snapshot_delete if isinstance(raw_snapshot_delete, bool) else False
529+
)
530+
raw_min_age = config.get("snapshot_delete_min_age_days", 7)
531+
snapshot_delete_min_age_days = (
532+
raw_min_age if isinstance(raw_min_age, int) else 7
533+
)
521534
raw_max_results = config.get("tool_search_max_results", 5)
522535
tool_search_max_results = (
523536
raw_max_results if isinstance(raw_max_results, int) else 5
@@ -675,6 +688,8 @@ def main() -> int:
675688
os.environ["ENABLE_AUTO_BACKUP"] = str(enable_auto_backup).lower()
676689
os.environ["AUTO_BACKUP_THROTTLE_MINUTES"] = str(auto_backup_throttle_minutes)
677690
os.environ["AUTO_BACKUP_RETAIN_PER_ENTITY"] = str(auto_backup_retain_per_entity)
691+
os.environ["ENABLE_SNAPSHOT_DELETE"] = str(enable_snapshot_delete).lower()
692+
os.environ["SNAPSHOT_DELETE_MIN_AGE_DAYS"] = str(snapshot_delete_min_age_days)
678693
# Persist saved custom tools across addon restarts. /data is the
679694
# per-addon writable directory mapped by Supervisor and survives
680695
# add-on updates (but not uninstall/reinstall — users should copy

homeassistant-addon/translations/en.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,21 @@ configuration:
100100
Maximum number of snapshots kept per entity. Older snapshots beyond
101101
this cap are rotated out on each successful capture. Default 100,
102102
range 1–10000.
103+
enable_snapshot_delete:
104+
name: Allow snapshot deletion
105+
description: >-
106+
Lets ha_manage_backup delete full HA snapshot tarballs
107+
(scope='snapshot', action='delete'). Off by default — a snapshot may
108+
be the last recovery point after a mistaken change, so a human must
109+
opt in here. Even when on, scheduled backups, the newest remaining
110+
snapshot, and anything younger than the age floor below stay
111+
protected.
112+
snapshot_delete_min_age_days:
113+
name: Minimum snapshot age to delete (days)
114+
description: >-
115+
A snapshot must be at least this old before it can be deleted. Range
116+
0–365; 0 disables the floor (the newest-snapshot and
117+
scheduled-backup protections still apply). Default 7.
103118
verify_ssl:
104119
name: Verify TLS certificate
105120
description: >-

src/ha_mcp/config.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,26 @@ class Settings(BaseSettings):
347347
7, ge=1, le=365, alias="HAMCP_AUTO_BACKUP_CALENDAR_LOOKAHEAD_DAYS"
348348
)
349349

350+
# Snapshot-tarball deletion gate (#1861). Off by default: an agent
351+
# deleting a full HA snapshot is categorically riskier than the
352+
# lightweight `edits`-scope auto-backups (which already delete freely),
353+
# since a snapshot may be the last recovery point after the agent
354+
# itself broke something. A human must opt in via env var, the web
355+
# settings UI override file, or (in the add-on) the Supervisor options
356+
# — never something the agent can flip on itself.
357+
enable_snapshot_delete: bool = Field(False, alias="ENABLE_SNAPSHOT_DELETE")
358+
359+
# Minimum age (days) a snapshot must have before it's deletable. This is
360+
# the load-bearing guard, not `enable_snapshot_delete`: a count-based
361+
# "keep the last N" rule is defeatable by an agent flooding new
362+
# snapshots before deleting old ones, but it cannot forge a backup's
363+
# HA-stamped creation date. 0 disables the age floor (still gated by
364+
# enable_snapshot_delete + the newest-snapshot / automatic-backup
365+
# guards enforced in tools/backup.py).
366+
snapshot_delete_min_age_days: int = Field(
367+
7, ge=0, le=365, alias="SNAPSHOT_DELETE_MIN_AGE_DAYS"
368+
)
369+
350370
# Mirror the legacy ``os.getenv("FLAG", "").lower() in ("true", ...)``
351371
# semantics for the ex-direct-getenv ``enable_filesystem_tools`` flag (and
352372
# its sibling toggles listed above): an empty env var value MUST be treated
@@ -1486,6 +1506,10 @@ def _reset_embedded_connection() -> None:
14861506
"HAMCP_AUTO_BACKUP_CALENDAR_LOOKAHEAD_DAYS",
14871507
int,
14881508
),
1509+
BackupOverrideField("enable_snapshot_delete", "ENABLE_SNAPSHOT_DELETE", bool),
1510+
BackupOverrideField(
1511+
"snapshot_delete_min_age_days", "SNAPSHOT_DELETE_MIN_AGE_DAYS", int
1512+
),
14891513
)
14901514

14911515
# Override-file location is the same data dir that holds tool_config.json
@@ -1640,6 +1664,13 @@ def _coerce_backup_int_value(field_name: str, raw: object) -> tuple[bool, Any]:
16401664
coerced,
16411665
)
16421666
return False, None
1667+
if field_name == "snapshot_delete_min_age_days" and not 0 <= coerced <= 365:
1668+
logger.warning(
1669+
"backup_settings.json: snapshot_delete_min_age_days=%d out of "
1670+
"range 0..365; ignoring",
1671+
coerced,
1672+
)
1673+
return False, None
16431674
return True, coerced
16441675

16451676

src/ha_mcp/settings_ui/_handlers_backups.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ async def _get_backup_config(
278278
"auto_backup_throttle_minutes": (0, 1440),
279279
"auto_backup_retain_per_entity": (1, 10_000),
280280
"auto_backup_calendar_lookahead_days": (1, 365),
281+
"snapshot_delete_min_age_days": (0, 365),
281282
}
282283

283284

src/ha_mcp/settings_ui/settings.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,6 +1053,14 @@ const BACKUP_FIELD_LABELS = {
10531053
label: 'Calendar lookahead (days)',
10541054
help: 'How far ahead to query for calendar events when capturing pre-edit snapshots. Range 1–365.',
10551055
},
1056+
enable_snapshot_delete: {
1057+
label: 'Allow snapshot deletion',
1058+
help: 'Lets ha_manage_backup delete full HA snapshot tarballs. Off by default: a snapshot may be the last recovery point after a mistaken change. Even when on, scheduled backups, the newest remaining snapshot, and anything younger than the age floor below stay protected.',
1059+
},
1060+
snapshot_delete_min_age_days: {
1061+
label: 'Minimum snapshot age to delete (days)',
1062+
help: 'A snapshot must be at least this old before it can be deleted. Range 0–365; 0 disables the floor (the newest-snapshot and scheduled-backup protections still apply).',
1063+
},
10561064
};
10571065

10581066
const BACKUP_ORIGIN_LABELS = {
@@ -1104,6 +1112,7 @@ function renderBackupConfig() {
11041112
let max = 10000;
11051113
if (f.field === 'auto_backup_throttle_minutes') { min = 0; max = 1440; }
11061114
else if (f.field === 'auto_backup_calendar_lookahead_days') { min = 1; max = 365; }
1115+
else if (f.field === 'snapshot_delete_min_age_days') { min = 0; max = 365; }
11071116
controlHtml = `<input type="number" name="backup:${escapeHtml(f.field)}" data-field="${escapeHtml(f.field)}" aria-labelledby="label-backup-${escapeHtml(f.field)}" value="${Number(f.value)}" min="${min}" max="${max}" ${f.editable ? '' : 'disabled'}>`;
11081117
}
11091118
let originMsg;

0 commit comments

Comments
 (0)