Skip to content

Commit 3b46f89

Browse files
committed
fix: ruff format, mypy, throttle baseline, pytest marks, annotation false-positive
- Fix throttle false-block on cold-start: skip throttle gate when no prior snapshot exists for the key (previously `get(key, 0.0)` made first capture compare against time-0, blocking when monotonic() < throttle). - Add type: ignore[import-untyped] for yaml imports (no stubs installed). - Remove unused type: ignore comments on auto_backup.py decorator path, clean up snap_domain typing to str. - Add Any return type annotation to _backup_mgr() helper. - Remove example function definitions from auto_backup.py module docstring — test_tool_annotations.py's regex was matching them as real tool defs. - Use registered pytest marks (script, convenience) instead of unregistered scripts/scenes/dashboards. - ruff format on all 27 changed py files.
1 parent 01cc5d9 commit 3b46f89

18 files changed

Lines changed: 843 additions & 478 deletions

homeassistant-addon/start.py

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ def generate_secret_path() -> str:
3939

4040

4141
_SECRET_PATH_RE = re.compile(r"^/(?!.*://)\S{7,}$")
42-
_SECRET_PATH_HINT = "Path must start with '/', contain no '://', and be at least 8 characters."
42+
_SECRET_PATH_HINT = (
43+
"Path must start with '/', contain no '://', and be at least 8 characters."
44+
)
4345

4446

4547
def _is_valid_secret_path(path: str) -> bool:
@@ -65,7 +67,9 @@ def get_or_create_secret_path(data_dir: Path, custom_path: str = "") -> str:
6567
if not path.startswith("/"):
6668
path = "/" + path
6769
if not _is_valid_secret_path(path):
68-
log_error(f"Custom secret path is invalid ({path!r}), ignoring. {_SECRET_PATH_HINT}")
70+
log_error(
71+
f"Custom secret path is invalid ({path!r}), ignoring. {_SECRET_PATH_HINT}"
72+
)
6973
else:
7074
log_info("Using custom secret path from configuration")
7175
# Update stored path for consistency
@@ -80,7 +84,9 @@ def get_or_create_secret_path(data_dir: Path, custom_path: str = "") -> str:
8084
log_info("Using existing auto-generated secret path")
8185
return stored_path
8286
elif stored_path:
83-
log_error(f"Stored secret path is invalid ({stored_path!r}), regenerating. {_SECRET_PATH_HINT}")
87+
log_error(
88+
f"Stored secret path is invalid ({stored_path!r}), regenerating. {_SECRET_PATH_HINT}"
89+
)
8490
else:
8591
log_error("Stored secret path is empty, regenerating")
8692
except Exception as e:
@@ -234,31 +240,59 @@ def main() -> int:
234240
backup_hint = config.get("backup_hint", "normal")
235241
custom_secret_path = config.get("secret_path", "")
236242
raw_tool_search = config.get("enable_tool_search", False)
237-
enable_tool_search = raw_tool_search if isinstance(raw_tool_search, bool) else False
243+
enable_tool_search = (
244+
raw_tool_search if isinstance(raw_tool_search, bool) else False
245+
)
238246
raw_yaml_config = config.get("enable_yaml_config_editing", False)
239-
enable_yaml_config_editing = raw_yaml_config if isinstance(raw_yaml_config, bool) else False
247+
enable_yaml_config_editing = (
248+
raw_yaml_config if isinstance(raw_yaml_config, bool) else False
249+
)
240250
raw_filesystem_tools = config.get("enable_filesystem_tools", False)
241-
enable_filesystem_tools = raw_filesystem_tools if isinstance(raw_filesystem_tools, bool) else False
242-
raw_custom_component = config.get("enable_custom_component_integration", False)
243-
enable_custom_component_integration = raw_custom_component if isinstance(raw_custom_component, bool) else False
251+
enable_filesystem_tools = (
252+
raw_filesystem_tools
253+
if isinstance(raw_filesystem_tools, bool)
254+
else False
255+
)
256+
raw_custom_component = config.get(
257+
"enable_custom_component_integration", False
258+
)
259+
enable_custom_component_integration = (
260+
raw_custom_component
261+
if isinstance(raw_custom_component, bool)
262+
else False
263+
)
244264
raw_code_mode = config.get("enable_code_mode", False)
245-
enable_code_mode = raw_code_mode if isinstance(raw_code_mode, bool) else False
265+
enable_code_mode = (
266+
raw_code_mode if isinstance(raw_code_mode, bool) else False
267+
)
246268
raw_lite_docstrings = config.get("enable_lite_docstrings", False)
247-
enable_lite_docstrings = raw_lite_docstrings if isinstance(raw_lite_docstrings, bool) else False
269+
enable_lite_docstrings = (
270+
raw_lite_docstrings if isinstance(raw_lite_docstrings, bool) else False
271+
)
248272
raw_auto_backup = config.get("enable_auto_backup", False)
249-
enable_auto_backup = raw_auto_backup if isinstance(raw_auto_backup, bool) else False
273+
enable_auto_backup = (
274+
raw_auto_backup if isinstance(raw_auto_backup, bool) else False
275+
)
250276
raw_throttle = config.get("auto_backup_throttle_minutes", 0)
251-
auto_backup_throttle_minutes = raw_throttle if isinstance(raw_throttle, int) else 0
277+
auto_backup_throttle_minutes = (
278+
raw_throttle if isinstance(raw_throttle, int) else 0
279+
)
252280
raw_retain = config.get("auto_backup_retain_per_entity", 20)
253-
auto_backup_retain_per_entity = raw_retain if isinstance(raw_retain, int) else 20
281+
auto_backup_retain_per_entity = (
282+
raw_retain if isinstance(raw_retain, int) else 20
283+
)
254284
raw_max_results = config.get("tool_search_max_results", 5)
255-
tool_search_max_results = raw_max_results if isinstance(raw_max_results, int) else 5
285+
tool_search_max_results = (
286+
raw_max_results if isinstance(raw_max_results, int) else 5
287+
)
256288
raw_disabled = config.get("disabled_tools", "")
257289
disabled_tools_raw = raw_disabled if isinstance(raw_disabled, str) else ""
258290
raw_pinned = config.get("pinned_tools", "")
259291
pinned_tools_raw = raw_pinned if isinstance(raw_pinned, str) else ""
260292
verify_ssl = resolve_bool_option(config, "verify_ssl", True)
261-
advanced_debug_logging = resolve_bool_option(config, "advanced_debug_logging", False)
293+
advanced_debug_logging = resolve_bool_option(
294+
config, "advanced_debug_logging", False
295+
)
262296
except Exception as e:
263297
log_error(f"Failed to read config: {e}, using defaults")
264298

@@ -288,7 +322,9 @@ def main() -> int:
288322
os.environ["ENABLE_TOOL_SEARCH"] = str(enable_tool_search).lower()
289323
os.environ["ENABLE_YAML_CONFIG_EDITING"] = str(enable_yaml_config_editing).lower()
290324
os.environ["HAMCP_ENABLE_FILESYSTEM_TOOLS"] = str(enable_filesystem_tools).lower()
291-
os.environ["HAMCP_ENABLE_CUSTOM_COMPONENT_INTEGRATION"] = str(enable_custom_component_integration).lower()
325+
os.environ["HAMCP_ENABLE_CUSTOM_COMPONENT_INTEGRATION"] = str(
326+
enable_custom_component_integration
327+
).lower()
292328
os.environ["ENABLE_CODE_MODE"] = str(enable_code_mode).lower()
293329
os.environ["ENABLE_LITE_DOCSTRINGS"] = str(enable_lite_docstrings).lower()
294330
os.environ["ENABLE_AUTO_BACKUP"] = str(enable_auto_backup).lower()
@@ -302,9 +338,7 @@ def main() -> int:
302338
# tool isn't registered anyway, so the file is never read or
303339
# written. Operators can override by setting CODE_MODE_SAVED_TOOLS_PATH
304340
# in the add-on's environment if they want a different location.
305-
os.environ.setdefault(
306-
"CODE_MODE_SAVED_TOOLS_PATH", "/data/saved_tools.json"
307-
)
341+
os.environ.setdefault("CODE_MODE_SAVED_TOOLS_PATH", "/data/saved_tools.json")
308342
os.environ["TOOL_SEARCH_MAX_RESULTS"] = str(tool_search_max_results)
309343
os.environ["DISABLED_TOOLS"] = disabled_tools_raw
310344
os.environ["PINNED_TOOLS"] = pinned_tools_raw
@@ -331,6 +365,7 @@ def main() -> int:
331365

332366
# Configure logging before server start (v3 removed log_level from run())
333367
import logging
368+
334369
logging.basicConfig(level=logging.INFO)
335370

336371
# Import and register browser landing before server start
@@ -353,6 +388,7 @@ def main() -> int:
353388
from ha_mcp.utils.kill_signal_diagnostics import (
354389
schedule_install_after_uvicorn,
355390
)
391+
356392
schedule_install_after_uvicorn()
357393
except Exception as e:
358394
log_error(f"advanced_debug_logging install failed: {e!r}; continuing")
@@ -364,7 +400,9 @@ def main() -> int:
364400
# server's actual FastMCP instance (not the _DeferredMCP wrapper)
365401
# so mypy doesn't trip over the duck-typed __getattr__ forwarding.
366402
server_instance = _get_server()
367-
register_settings_routes(server_instance.mcp, server_instance, secret_path=secret_path)
403+
register_settings_routes(
404+
server_instance.mcp, server_instance, secret_path=secret_path
405+
)
368406
logging.getLogger("mcp.server.streamable_http").addFilter(
369407
StatelessSessionLogFilter()
370408
)
@@ -391,7 +429,9 @@ def main() -> int:
391429
cause = e.__cause__ or e.__context__
392430
if cause:
393431
log_error(f"Caused by: {cause}")
394-
traceback.print_exception(type(cause), cause, cause.__traceback__, file=sys.stderr)
432+
traceback.print_exception(
433+
type(cause), cause, cause.__traceback__, file=sys.stderr
434+
)
395435
if isinstance(e, SystemExit):
396436
return int(e.code) if isinstance(e.code, int) else 1
397437
return 1

src/ha_mcp/backup_manager.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
from pathlib import Path
5050
from typing import Any
5151

52-
import yaml
52+
import yaml # type: ignore[import-untyped]
5353

5454
logger = logging.getLogger(__name__)
5555

@@ -208,7 +208,16 @@ async def maybe_snapshot(
208208
async with lock:
209209
now = time.monotonic()
210210
throttle = self.throttle_seconds
211-
if throttle and (now - self._last_snapshot.get(key, 0.0)) < throttle:
211+
# Skip throttle if no prior snapshot exists for this key.
212+
# Using ``get(key, 0.0)`` would falsely block the first capture
213+
# whenever ``monotonic()`` < throttle (typical on a fresh process
214+
# in CI), since 0.0 would be treated as "last snapshot at
215+
# monotonic time 0".
216+
if (
217+
throttle
218+
and key in self._last_snapshot
219+
and (now - self._last_snapshot[key]) < throttle
220+
):
212221
return None
213222
try:
214223
config = await handler.fetch(self._client, entity_id)

src/ha_mcp/settings_ui.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,7 +1194,7 @@ async def _settings_info(_: Request) -> JSONResponse:
11941194

11951195
# ---- Auto-backup routes (#1288) ----
11961196

1197-
def _backup_mgr():
1197+
def _backup_mgr() -> Any:
11981198
settings = get_global_settings()
11991199
client = getattr(server, "client", None) or getattr(server, "_client", None)
12001200
return get_backup_manager(client, settings) if client is not None else None
@@ -1258,7 +1258,7 @@ async def _view_backup(request: Request) -> JSONResponse:
12581258
async def _diff_backup(request: Request) -> JSONResponse:
12591259
import difflib
12601260

1261-
import yaml
1261+
import yaml # type: ignore[import-untyped]
12621262

12631263
mgr = _backup_mgr()
12641264
if mgr is None:

src/ha_mcp/tools/auto_backup.py

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,16 @@
55
66
Usage
77
-----
8-
Simple case (entity ID lives in a single kwarg)::
8+
Simple case (entity ID lives in a single kwarg): wrap the existing tool
9+
function with ``@with_auto_backup(domain="<domain>", id_param="<kwarg>")``
10+
placed above the ``@mcp.tool``/``@tool`` decorator and below it the
11+
``@log_tool_usage`` line, so the order from outermost to innermost is
12+
``@tool`` -> ``@with_auto_backup`` -> ``@log_tool_usage`` -> the async def.
913
10-
@with_auto_backup(domain="automation", id_param="identifier")
11-
@mcp.tool(name="ha_config_set_automation", ...)
12-
@log_tool_usage
13-
async def ha_config_set_automation(self, identifier: str, ...): ...
14-
15-
Computed-key case (helpers — domain encodes ``helper_type``)::
16-
17-
@with_auto_backup(
18-
domain_fn=lambda kw: f"helper_{kw['helper_type']}",
19-
id_fn=lambda kw: f"{kw['helper_type']}:{kw.get('helper_id', '')}",
20-
)
14+
Computed-key case (helpers — domain encodes ``helper_type``): use
15+
``domain_fn`` / ``id_fn`` instead of ``domain`` / ``id_param``; both take
16+
a single ``kw`` dict argument and return a string. Helpers typically use
17+
``domain_fn=lambda kw: f"helper_{kw['helper_type']}"``.
2118
2219
The decorator must be applied **above** ``@mcp.tool`` so FastMCP sees the
2320
final wrapped callable as the tool. ``functools.wraps`` preserves the
@@ -87,13 +84,13 @@ async def wrapper(*args: Any, **kwargs: Any) -> Any:
8784
args[0], "client", None
8885
)
8986
if client_obj is not None:
90-
snap_domain = (
91-
domain_fn(kwargs) if domain_fn is not None else domain # type: ignore[assignment]
87+
snap_domain: str = (
88+
domain_fn(kwargs) if domain_fn is not None else domain or ""
9289
)
9390
if id_fn is not None:
9491
entity_id = _resolve_str(id_fn(kwargs))
9592
else:
96-
entity_id = _resolve_str(kwargs.get(id_param)) # type: ignore[arg-type]
93+
entity_id = _resolve_str(kwargs.get(id_param or ""))
9794
if entity_id:
9895
mgr = get_backup_manager(client_obj, settings)
9996
await mgr.maybe_snapshot(

0 commit comments

Comments
 (0)