Skip to content

Commit 9c79df4

Browse files
kingpanther13claude
andcommitted
refactor(dev): harvest update_source's preserved overrides from the flow schema
Replaces the hardcoded _PRESERVED_OPTION_KEYS tuple with a schema-driven harvest (description.suggested_value, the same suggestion-over-default rule as the walker's keep_current_values backfill), so a field added to the component's options flow later cannot be silently wiped by a partial update_source submit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr
1 parent b47cb58 commit 9c79df4

2 files changed

Lines changed: 55 additions & 27 deletions

File tree

src/ha_mcp/tools/tools_dev.py

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@
3737
invalidate_caps,
3838
is_unknown_command,
3939
)
40+
from .config_entry_flow_form import (
41+
_MISSING_DEFAULT,
42+
_step_owned_submission_value,
43+
)
4044
from .helpers import (
4145
exception_to_structured_error,
4246
log_tool_usage,
@@ -71,25 +75,8 @@
7175
# (custom_components/ha_mcp_tools/const.py OPT_CHANNEL / OPT_PIP_SPEC).
7276
_OPT_CHANNEL = "channel"
7377
_OPT_PIP_SPEC = "pip_spec"
74-
_OPT_SERVER_URL = "server_url"
75-
_OPT_EXTERNAL_URL = "external_url"
76-
_OPT_WEBHOOK_ID_OVERRIDE = "webhook_id_override"
77-
_OPT_SECRET_PATH_OVERRIDE = "secret_path_override"
7878
_VALID_CHANNELS = ("stable", "dev")
7979

80-
# Optional text fields the component's options flow pre-fills via
81-
# suggested_value (so the UI can clear them). Because an OMITTED optional field
82-
# reads as "cleared" rather than "unchanged", a partial update_source submit
83-
# must resend these at their current values or it would blank the user's
84-
# server-URL / connect-secret overrides.
85-
_PRESERVED_OPTION_KEYS = (
86-
_OPT_PIP_SPEC,
87-
_OPT_SERVER_URL,
88-
_OPT_EXTERNAL_URL,
89-
_OPT_WEBHOOK_ID_OVERRIDE,
90-
_OPT_SECRET_PATH_OVERRIDE,
91-
)
92-
9380
# Delay before a self-affecting action (embedded entry reload / options
9481
# submit) fires, so this tool's JSON response flushes to the MCP client
9582
# before the serving thread is torn down. Mirrors
@@ -368,17 +355,43 @@ def _fields_from_flow_schema(flow: dict[str, Any]) -> dict[str, Any]:
368355
}
369356

370357

358+
def _preserved_flow_overrides(flow: dict[str, Any]) -> dict[str, Any]:
359+
"""Overrides an open server-entry flow carries stored values for.
360+
361+
A partial ``update_source`` submit must resend these or it would blank
362+
them: an omitted optional field reads as "cleared", not "unchanged". The
363+
resend is harvested from ``description.suggested_value`` on the flow's own
364+
schema — the clearable text fields, per the same suggestion-over-default
365+
rule as the generic walker's ``keep_current_values`` backfill (issue
366+
#2254) — rather than a hardcoded key list, so a field added to the
367+
component's options flow later cannot be silently wiped here. ``channel``
368+
carries only a schema default (= its current value), never a suggestion,
369+
so it drops out naturally; an empty or cleared override carries no
370+
resendable value and stays omitted, which is how a cleared field stays
371+
cleared.
372+
"""
373+
preserved: dict[str, Any] = {}
374+
for item in flow.get("data_schema") or []:
375+
if not isinstance(item, dict) or not item.get("name"):
376+
continue
377+
stored = _step_owned_submission_value(item)
378+
if stored is _MISSING_DEFAULT or stored in (None, ""):
379+
continue
380+
preserved[str(item["name"])] = stored
381+
return preserved
382+
383+
371384
async def _open_server_entry_flow(
372385
client: Any, entry_id: str
373386
) -> tuple[str, dict[str, Any], dict[str, Any]] | None:
374387
"""Open the options flow for a KNOWN server ``entry_id``; ``None`` on failure.
375388
376389
Builds ``current_options`` from the freshly-opened flow's own schema (via
377390
``_fields_from_flow_schema``) rather than the component's narrower
378-
``{channel, pip_spec}`` shape, so callers like ``_update_source`` that
379-
resend ``_PRESERVED_OPTION_KEYS`` (``server_url`` / ``external_url`` /
380-
``webhook_id_override`` / ``secret_path_override`` — fields the
381-
``server_entry`` capability does not carry) still see them.
391+
``{channel, pip_spec}`` shape, so callers like ``_update_source`` — whose
392+
preserved-overrides resend harvests the flow schema itself (``server_url``
393+
/ ``external_url`` / ``webhook_id_override`` / ``secret_path_override`` —
394+
fields the ``server_entry`` capability does not carry) still see them.
382395
"""
383396
try:
384397
flow = await client.start_options_flow(entry_id)
@@ -2047,12 +2060,7 @@ async def _update_source(
20472060
)
20482061
entry_id, flow, current = found
20492062

2050-
# Resend the user's current overrides (see _PRESERVED_OPTION_KEYS) so a
2051-
# channel/pip-spec change here does not blank them — an omitted optional
2052-
# field reads as "cleared", not "unchanged".
2053-
user_input: dict[str, Any] = {
2054-
key: current[key] for key in _PRESERVED_OPTION_KEYS if current.get(key)
2055-
}
2063+
user_input: dict[str, Any] = _preserved_flow_overrides(flow)
20562064
if channel is not None:
20572065
user_input[_OPT_CHANNEL] = channel
20582066
if pip_spec is not None:

tests/src/unit/test_tools_dev.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,26 @@ async def test_update_source_preserves_url_and_secret_overrides(self):
464464
},
465465
)
466466

467+
async def test_update_source_preserves_fields_beyond_the_known_overrides(self):
468+
# The resend is harvested from the flow schema itself, not a hardcoded
469+
# key list, so a field the component's options flow grows later is
470+
# preserved without this module learning its name. A bare-default
471+
# field (channel) still drops out — omission keeps it — and a cleared
472+
# override carries no resendable value, so it stays omitted.
473+
flow = dict(_SERVER_FLOW_WITH_OVERRIDES)
474+
flow["data_schema"] = list(flow["data_schema"]) + [
475+
{"name": "future_override", "description": {"suggested_value": "kept"}},
476+
{"name": "cleared_override", "description": {"suggested_value": ""}},
477+
]
478+
client = _mock_client(entries=[{"entry_id": "server-e"}], flows=[flow])
479+
await DevTools(client).ha_dev_manage_server(
480+
action="update_source", channel="stable"
481+
)
482+
submitted = client.submit_options_flow_step.await_args.args[1]
483+
assert submitted["future_override"] == "kept"
484+
assert "cleared_override" not in submitted
485+
assert submitted["channel"] == "stable"
486+
467487
async def test_update_source_new_pip_spec_wins_over_preserved(self):
468488
# A caller-supplied pip_spec must override the preserved (suggested_value)
469489
# pin, not be clobbered by it: the preserve dict is seeded first, then the

0 commit comments

Comments
 (0)