Skip to content

Commit 8944595

Browse files
fix: Schedule the HACS refresh nudge via the FastMCP lifespan (#2172)
* fix: Schedule the HACS refresh nudge via the FastMCP lifespan The startup nudge was scheduled in __main__'s _run_with_shutdown, which the CLI entry points share — but the HA add-on's start.py calls mcp.run() directly and never passes through it, so the nudge never ran on the add-on, the deployment it was built for. Found on a live HAOS instance; CI could not see it because the e2e suites launch via the CLI entry points. Attach the task as the server's FastMCP lifespan instead: the lifespan manager is entered by every transport runner (stdio and the HTTP app factories), ref-counted to fire once per run, so one site covers stdio, web, oauth, oidc, and the add-on launcher. The lifespan's exit cancels a nudge parked in a retry sleep, replacing the _cancel_tasks wiring. * test: Prove the startup nudge end to end through a real launcher Boot the actual ha-mcp stdio binary against the e2e container and watch for the refresh marker in its data dir — the observable end of the process -> lifespan -> task -> WebSocket -> HACS -> marker chain. This is the regression shape of the add-on gap: the unit suite pins the lifespan wiring, but only a real launcher shows the chain completing, which is exactly what stayed invisible to CI when the scheduling site was launcher-specific. * chore: Allowlist the lifespan's cancelled-task await in the CodeQL gate Same py/ineffectual-statement false positive as the embedded teardown entries: the bare await inside contextlib.suppress drives the cancelled nudge task to completion before the server lifespan exits. * fix(internal): Log a due startup nudge before any WebSocket work One unconditional INFO line when a pass is due, emitted ahead of the first WebSocket call: it proves the launcher scheduled the nudge even where HACS is absent and the pass ends silently — the observable the HAOS add-on lane greps for, and the line whose absence exposed the launcher gap. The not-due hot path stays log-silent; both sides pinned in the unit suite. * test: Cover every launcher lane's startup-nudge contract - ha-mcp-web positive lane: the HTTP lifespan fires at uvicorn startup with no client, and the marker appears. - ha-mcp-oauth negative lane: the pass returns at the OAuth-sentinel gate — no past-the-gate log lines, no marker — instead of burning the retry schedule on auth failures every boot. - Embedded negative lane: the in-process server writes no marker (the is_embedded gate; the component's own hacs_nudge covers embedded). - HAOS add-on lane: the inaddon tier greps the dev add-on's own logs for the due-pass INFO line — the real launcher that had the gap. - ha-mcp-oidc deliberately has no lane: it exits without HA credentials at startup, so it has no sentinel state to prove, and its run path is the same call the web lane drives. - stdio lane: keep_alive=False so the subprocess ends with the client context instead of leaking past it (review finding). - Backend dispatch skip ceilings bumped for the two new marker-gated tests. * chore: Scope CodeQL allowlist entries to the flagged statement Generic-message rules (py/ineffectual-statement) previously suppressed path-wide. Entries now carry an optional code substring matched against the flagged line's text read from the checkout — only the named statement is suppressed, any other finding of the rule in the file still fails the gate, and content matching means edits above the statement don't churn the allowlist. Fails closed when the source line cannot be read (review finding). * fix: Make the inaddon nudge probe restart-first; address review findings The inaddon lane failure was the probe, not the feature: the run's diagnostics artifact shows the due-pass line in the add-on log — the lifespan fired on the real add-on launcher — but the test ran at the end of the suite, by which point the one-shot boot line sat thousands of request-log lines beyond even the expanded journald search window. The test now self-restarts the add-on first (the test_addon_debug_log_level pattern: settings restart endpoint, fresh connection per poll, shared client warmed back up) so the line is in the fresh tail, and probes with safe_call_tool so a briefly unavailable Supervisor log endpoint counts as a failed poll instead of aborting the wait. Also from review: - CodeQL allowlist entry for the e2e helper's cancelled-racer await (same statement-scoped false-positive class). - haos skip-ceiling comment arithmetic corrected (was 46, not 47). - Negative-lane marker windows widened to 10 s with comments anchoring them to first-attempt completion, not the retry schedule. - The unreachable-HACS unit test now asserts the due-pass line logged although every WS attempt failed — the strongest before-any-WS-work proof, and the property the add-on lane depends on. * test: Use MCPAssertions for the inaddon slug lookup call_tool_success per the test conventions: a failed ha_get_addon listing reports itself instead of reading as a missing dev add-on. * fix: Make the inaddon nudge probe marker-state-independent The restart-first probe assumed a fresh add-on boot is always due; the run's diagnostics disproved that — the add-on's /data persists, an early boot that lives ~8.5 minutes completes its HACS-absent pass and writes the marker, and every later boot is legitimately not due (50 boots, one due-line). The not-due return now emits a DEBUG line, and the test drives the add-on to DEBUG via the settings flow, restarts, and accepts either per-boot line as scheduling proof, restoring INFO after. Budgets sized inside pytest-timeout's 300 s cap, which the previous version could exceed and mask its own failure. * test: Probe the shared-client warmup with safe_call_tool A ToolError raised mid-bounce is not in the transient set and would escape the warm-up loop; probe with safe_call_tool and break only on a successful payload, raising a named assertion at the deadline. * style: Explain the transient pass in the warmup probe (py/empty-except) * test: Give the inaddon nudge probe real HAOS budgets The 60 s warm-up starved in CI: the flow does two back-to-back container restarts, and DEBUG-level logging makes every Supervisor log fetch heavy. Budgets now reflect observed recovery times (probe 180, restore 120, warm 180) with a per-test pytest.mark.timeout sized to the phase sum plus restart margin, per the other HAOS long-runners. * test: Warm the shared client on any completed round-trip Payload inspection in the warm-up proved harmful — it kept the loop spinning against a healthy session through two CI rounds. A completed round-trip, success or ToolError, is what proves the shared session usable again (the debug-log-level precedent); only transport-level transients retry, and the deadline assertion now names the last error. The probe also post-filters for the two real per-boot phrases so its own DEBUG-logged search argument cannot satisfy it. * test: Bound every await in the inaddon nudge probe Round-4 CI hit pytest-timeout with none of the test's own deadlines firing: the event loop sat idle at selector.select on an unbounded streamable-HTTP read against the bouncing addon. Every MCP exchange — fresh-client probe calls and the shared-client warmup — now runs under asyncio.wait_for(30); TimeoutError is in the transient set, so a bound trip is retried and the phase deadlines actually enforce. * test: Warm-up via safe_call_tool, deadline-capped iterations Both from review: safe_call_tool inside the bound (a returned dict of either shape is the completed round-trip warm-up needs), and the last iteration's wait and sleep are capped to the remaining budget so the loop cannot overshoot _WARM_TIMEOUT. * test: Pin HACS startup nudge boot evidence * test: Stabilize the nudge log baseline * test: Bind nudge proof to restarted process --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 47a20f1 commit 8944595

12 files changed

Lines changed: 1154 additions & 72 deletions

scripts/codeql_quality_gate.py

Lines changed: 123 additions & 13 deletions
Large diffs are not rendered by default.

src/ha_mcp/__main__.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -666,13 +666,6 @@ async def _run_with_shutdown(server_coro: Coroutine[Any, Any, Any]) -> None:
666666
server_task = asyncio.create_task(server_coro)
667667
shutdown_task = asyncio.create_task(_shutdown_event.wait())
668668

669-
# Fire-and-forget: ask HACS to surface a paired component update after a
670-
# server update (advisory; see hacs_auto_refresh). Not in the wait set —
671-
# its completion must not stop the server.
672-
from ha_mcp.hacs_auto_refresh import maybe_refresh_hacs_after_update
673-
674-
hacs_refresh_task = asyncio.create_task(maybe_refresh_hacs_after_update())
675-
676669
try:
677670
done, pending = await asyncio.wait(
678671
[server_task, shutdown_task],
@@ -732,7 +725,7 @@ async def _run_with_shutdown(server_coro: Coroutine[Any, Any, Any]) -> None:
732725
logger.warning("Resource cleanup timed out")
733726

734727
try:
735-
await _cancel_tasks(server_task, shutdown_task, hacs_refresh_task)
728+
await _cancel_tasks(server_task, shutdown_task)
736729
except Exception as e:
737730
# Teardown must never mask the exception being propagated from the
738731
# try block (Python drops the original if finally raises).

src/ha_mcp/hacs_auto_refresh.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import hashlib
2626
import json
2727
import logging
28+
from collections.abc import AsyncIterator
29+
from contextlib import asynccontextmanager, suppress
2830
from pathlib import Path
2931
from typing import Any
3032

@@ -222,8 +224,28 @@ async def maybe_refresh_hacs_after_update() -> None:
222224
info = await asyncio.to_thread(get_update_info)
223225
marker = await asyncio.to_thread(_read_marker, ha_url)
224226
if not _nudge_due(current, info, marker):
227+
# DEBUG, not INFO: the not-due return is the per-conversation
228+
# stdio hot path and must stay quiet at default levels. The line
229+
# exists so a DEBUG-level launcher (the HAOS add-on lane) can
230+
# prove scheduling even when a completed earlier pass makes every
231+
# later boot legitimately not due.
232+
logger.debug(
233+
"HACS auto-refresh: pass not due (marker current for server %s)",
234+
current,
235+
)
225236
return
226237

238+
# The one unconditional line a due pass emits BEFORE any WebSocket
239+
# work: it proves the launcher scheduled the nudge even where HACS
240+
# is absent and the pass ends silently — the observable the HAOS
241+
# add-on lane greps for, and the line whose absence exposed the
242+
# launcher gap this module's lifespan wiring closed.
243+
logger.info(
244+
"HACS auto-refresh: startup pass due (server %s); "
245+
"asking HACS for repository state",
246+
current,
247+
)
248+
227249
result = await _refresh_with_retries()
228250
if result is None:
229251
logger.debug(
@@ -250,3 +272,26 @@ async def maybe_refresh_hacs_after_update() -> None:
250272
raise
251273
except Exception:
252274
logger.debug("HACS auto-refresh nudge skipped", exc_info=True)
275+
276+
277+
@asynccontextmanager
278+
async def hacs_refresh_lifespan(_server: Any) -> AsyncIterator[dict[str, Any]]:
279+
"""Schedule the startup nudge for the lifetime of any server run.
280+
281+
Attached as the FastMCP ``lifespan`` so it runs on EVERY launcher —
282+
stdio, the HTTP CLI entry points, and the add-on's ``start.py``, which
283+
calls ``mcp.run()`` directly and never passes through ``__main__``'s
284+
``_run_with_shutdown`` (the wiring this replaces; the add-on gap was
285+
found live, not by CI, because the e2e suites launch via the CLI
286+
entry points).
287+
"""
288+
task = asyncio.create_task(maybe_refresh_hacs_after_update())
289+
try:
290+
yield {}
291+
finally:
292+
# The nudge may be mid-retry-sleep; a cancelled task must not
293+
# stall shutdown. maybe_refresh_hacs_after_update re-raises
294+
# CancelledError by design, so this await returns promptly.
295+
task.cancel()
296+
with suppress(asyncio.CancelledError):
297+
await task

src/ha_mcp/server.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from .config import _PACKAGE_VERSION, get_global_settings
2222
from .errors import ErrorCode, create_error_response
23+
from .hacs_auto_refresh import hacs_refresh_lifespan
2324
from .tools.helpers import raise_tool_error
2425
from .transforms import DEFAULT_PINNED_TOOLS
2526

@@ -142,6 +143,7 @@ def __init__(
142143
version=server_version,
143144
icons=SERVER_ICONS,
144145
instructions=instructions,
146+
lifespan=hacs_refresh_lifespan,
145147
)
146148

147149
# Register all tools and expert prompts

tests/src/e2e/basic/test_backend_dispatch_smoke.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@
8484
# gain 10; container is unchanged because the tests run there.
8585
# Entries below are CI-observed item counts, bumped only for intentional
8686
# marker-gated additions rather than runtime skips.
87-
"container": 72, # was 71; +1 Puppet-management test (haos_only + inaddon_only)
88-
"haos": 46, # was 45; +1 py3.14 invalidate_caches recovery e2e (container_only)
89-
"haos_inaddon": 75, # was 74; +1 inline dashboard_resource auto-backup e2e (external_only, #2060)
87+
"container": 73, # was 72; +1 inaddon startup-nudge e2e (haos_only + inaddon_only)
88+
"haos": 48, # was 46; +1 embedded HACS-nudge skip e2e (container_only), +1 inaddon startup-nudge e2e (inaddon_only)
89+
"haos_inaddon": 76, # was 75; +1 embedded HACS-nudge skip e2e (container_only)
9090
# Embedded backend (#1527, E2E_BACKEND=embedded). Skips exactly the container
9191
# lane's marker-skips PLUS two embedded-specific additions:
9292
# - haos_only + inaddon_only tests skip on embedded just like on container
@@ -100,7 +100,7 @@
100100
# 1) + not_on_embedded 2 = 101. Initially set to 115 as a buffer for
101101
# parametrize item-inflation; round 6 (run 28709196071) observed the exact
102102
# item count and the entry below is pinned to it.
103-
"embedded": 130, # was 129; +1 inline dashboard_resource auto-backup e2e (external_only, #2060)
103+
"embedded": 132, # was 130; +1 embedded HACS-nudge skip e2e (not_on_embedded), +1 inaddon startup-nudge e2e (haos_only + inaddon_only)
104104
# HAOS embedded backend (#1527, HAOS_TEST_MODE=embedded). A HAOS lane, so it
105105
# skips the SAME set as the external HAOS lane (container_only + inaddon_only)
106106
# PLUS two haos_embedded-specific additions:
@@ -118,7 +118,7 @@
118118
# lanes show (haos def 30 → ~35 observed; haos_inaddon def 50 → ~58) gives
119119
# ~84; initially set to 90 with a small buffer, and round 8 observed
120120
# exactly 90 — the entry below is pinned to the observed count.
121-
"haos_embedded": 104, # was 103; +1 inline dashboard_resource auto-backup e2e (external_only, #2060)
121+
"haos_embedded": 106, # was 104; +1 embedded HACS-nudge skip e2e (container_only), +1 inaddon startup-nudge e2e (inaddon_only)
122122
}
123123

124124

0 commit comments

Comments
 (0)